CVE-2022-29188: Server-Side Request Forgery (SSRF) (CWE-918)
value: Smokescreen is an HTTP proxy. The primary use case for Smokescreen is to prevent server-side request forgery (SSRF) attacks in which external attackers leverage the behavior of applications to connect to or scan internal infrastructure. Smokescreen also offers an option to deny access to additional (e.g., external) URLs by way of a deny list. There was an issue in Smokescreen that made it possible to bypass the deny list feature by surrounding the hostname with square brackets (e.g. `[example.com]`). This only impacted the HTTP proxy functionality of Smokescreen. HTTPS requests were not impacted. Smokescreen version 0.0.4 contains a patch for this issue.
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 6.5
Published: 2022-05-21T00:15Z
Repository: https://github.com/stripe/smokescreen
Language: Go
Files changed: pkg/smokescreen/smokescreen.go, pkg/smokescreen/smokescreen_test.go, pkg/smokescreen/testdata/acl.yaml
Commit date: 2022-04-27 19:11:50 -0700
Commit message: Fix hostname parsing for square brackets

Diff: diff --git a/pkg/smokescreen/smokescreen.go b/pkg/smokescreen/smokescreen.go
index f188768..b64de9f 100644
--- a/pkg/smokescreen/smokescreen.go
+++ b/pkg/smokescreen/smokescreen.go
@@ -455,11 +455,11 @@ func BuildProxy(config *Config) *goproxy.ProxyHttpServer {
 
 		// Set this on every request as every request mints a new goproxy.ProxyCtx
 		pctx.RoundTripper = rtFn
 
 		// Build an address parsable by net.ResolveTCPAddr
-		remoteHost := req.Host
+		remoteHost := req.URL.Hostname()
 		if strings.LastIndex(remoteHost, ":") <= strings.LastIndex(remoteHost, "]") {
 			switch req.URL.Scheme {
 			case "http":
 				remoteHost = net.JoinHostPort(remoteHost, "80")
 			case "https":
@@ -598,11 +598,11 @@ func logProxy(config *Config, pctx *goproxy.ProxyCtx) {
 
 func handleConnect(config *Config, pctx *goproxy.ProxyCtx) error {
 	sctx := pctx.UserData.(*smokescreenContext)
 
 	// Check if requesting role is allowed to talk to remote
-	sctx.decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, pctx.Req.Host)
+	sctx.decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, pctx.Req.URL.Hostname())
 	if pctx.Error != nil {
 		return pctx.Error
 	}
 	if !sctx.decision.allow {
 		return denyError{errors.New(sctx.decision.reason)}
diff --git a/pkg/smokescreen/smokescreen_test.go b/pkg/smokescreen/smokescreen_test.go
index a0c4d94..bbc701d 100644
--- a/pkg/smokescreen/smokescreen_test.go
+++ b/pkg/smokescreen/smokescreen_test.go
@@ -105,15 +105,15 @@ func TestClassifyAddr(t *testing.T) {
 			t.Errorf("Misclassified IP (%s): should be %s, but is instead %s.", localIP, test.expected, got)
 		}
 	}
 }
 
-func TestUnsafeAllowPrivateRanges (t *testing.T) {
+func TestUnsafeAllowPrivateRanges(t *testing.T) {
 	a := assert.New(t)
 
 	conf := NewConfig()
-	a.NoError(conf.SetDenyRanges([]string {"192.168.0.0/24", "10.0.0.0/8"}))
+	a.NoError(conf.SetDenyRanges([]string{"192.168.0.0/24", "10.0.0.0/8"}))
 	conf.ConnectTimeout = 10 * time.Second
 	conf.ExitTimeout = 10 * time.Second
 	conf.AdditionalErrorMessageOnDeny = "Proxy denied"
 
 	conf.UnsafeAllowPrivateRanges = true
@@ -158,11 +158,10 @@ func TestUnsafeAllowPrivateRanges (t *testing.T) {
 		if got != test.expected {
 			t.Errorf("Misclassified IP (%s): should be %s, but is instead %s.", localIP, test.expected, got)
 		}
 	}
 
-
 }
 
 // TestClearsErrors tests that we are correctly preserving/removing the X-Smokescreen-Error header.
 // This header is used to provide more granular errors to proxy clients, and signals that
 // there was an issue connecting to the proxy target.
@@ -441,10 +440,56 @@ func TestInvalidHost(t *testing.T) {
 			}
 		})
 	}
 }
 
+var hostSquareBracketsCases = []struct {
+	scheme    string
+	proxyType string
+}{
+	{"http", "http"},
+	{"https", "connect"},
+}
+
+func TestHostSquareBrackets(t *testing.T) {
+	for _, testCase := range hostSquareBracketsCases {
+		t.Run(testCase.scheme, func(t *testing.T) {
+			a := assert.New(t)
+			r := require.New(t)
+
+			cfg, err := testConfig("test-open-srv")
+			require.NoError(t, err)
+			logHook := proxyLogHook(cfg)
+
+			proxySrv := proxyServer(cfg)
+			defer proxySrv.Close()
+
+			// Create a http.Client that uses our proxy
+			client, err := proxyClient(proxySrv.URL)
+			r.NoError(err)
+
+			resp, err := client.Get(fmt.Sprintf("%s://[stripe.com]", testCase.scheme))
+			if err != nil {
+				r.Contains(err.Error(), "Request rejected by proxy")
+			} else {
+				r.Equal(http.StatusProxyAuthRequired, resp.StatusCode)
+			}
+
+			entry := findCanonicalProxyDecision(logHook.AllEntries())
+			r.NotNil(entry)
+
+			if a.Contains(entry.Data, "allow") {
+				a.Equal(false, entry.Data["allow"])
+				a.Equal("host matched rule in global deny list", entry.Data["decision_reason"])
+			}
+			if a.Contains(entry.Data, "proxy_type") {
+				a.Contains(entry.Data["proxy_type"], testCase.proxyType)
+			}
+		})
+	}
+}
+
 func TestErrorHeader(t *testing.T) {
 	a := assert.New(t)
 	r := require.New(t)
 
 	cfg, err := testConfig("test-trusted-srv")
diff --git a/pkg/smokescreen/testdata/acl.yaml b/pkg/smokescreen/testdata/acl.yaml
index 185f2e7..a55ffa4 100644
--- a/pkg/smokescreen/testdata/acl.yaml
+++ b/pkg/smokescreen/testdata/acl.yaml
@@ -1,14 +1,20 @@
 ---
-    version: v1
-    services:
-      - name: test-trusted-srv
-        project: security
-        action: enforce
-        allowed_domains:
-          - notarealhost.test
-          - httpbin.org
-      - name: test-local-srv
-        project: security
-        action: open
-        allowed_domains:
-          - 127.0.0.1
+version: v1
+services:
+  - name: test-trusted-srv
+    project: security
+    action: enforce
+    allowed_domains:
+      - notarealhost.test
+      - httpbin.org
+  - name: test-local-srv
+    project: security
+    action: open
+    allowed_domains:
+      - 127.0.0.1
+  - name: test-open-srv
+    project: security
+    action: open
+
+global_deny_list:
+  - stripe.com

Vulnerable code: 		remoteHost := req.Host
	sctx.decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, pctx.Req.Host)
func TestUnsafeAllowPrivateRanges (t *testing.T) {
	a.NoError(conf.SetDenyRanges([]string {"192.168.0.0/24", "10.0.0.0/8"}))

    version: v1
    services:
      - name: test-trusted-srv
        project: security
        action: enforce
        allowed_domains:
          - notarealhost.test
          - httpbin.org
      - name: test-local-srv
        project: security
        action: open
        allowed_domains:
          - 127.0.0.1
Fixed code: 		remoteHost := req.URL.Hostname()
	sctx.decision, sctx.lookupTime, pctx.Error = checkIfRequestShouldBeProxied(config, pctx.Req, pctx.Req.URL.Hostname())
func TestUnsafeAllowPrivateRanges(t *testing.T) {
	a.NoError(conf.SetDenyRanges([]string{"192.168.0.0/24", "10.0.0.0/8"}))
var hostSquareBracketsCases = []struct {
	scheme    string
	proxyType string
}{
	{"http", "http"},
	{"https", "connect"},
}

func TestHostSquareBrackets(t *testing.T) {
	for _, testCase := range hostSquareBracketsCases {
		t.Run(testCase.scheme, func(t *testing.T) {
			a := assert.New(t)
			r := require.New(t)

			cfg, err := testConfig("test-open-srv")
			require.NoError(t, err)
			logHook := proxyLogHook(cfg)

			proxySrv := proxyServer(cfg)
			defer proxySrv.Close()

			// Create a http.Client that uses our proxy
			client, err := proxyClient(proxySrv.URL)
			r.NoError(err)

			resp, err := client.Get(fmt.Sprintf("%s://[stripe.com]", testCase.scheme))
			if err != nil {
				r.Contains(err.Error(), "Request rejected by proxy")
			} else {
				r.Equal(http.StatusProxyAuthRequired, resp.StatusCode)
			}

			entry := findCanonicalProxyDecision(logHook.AllEntries())
			r.NotNil(entry)

			if a.Contains(entry.Data, "allow") {
				a.Equal(false, entry.Data["allow"])
				a.Equal("host matched rule in global deny list", entry.Data["decision_reason"])
			}
			if a.Contains(entry.Data, "proxy_type") {
				a.Contains(entry.Data["proxy_type"], testCase.proxyType)
			}
		})
	}
}

version: v1
services:
  - name: test-trusted-srv
    project: security
    action: enforce
    allowed_domains:
      - notarealhost.test
      - httpbin.org
  - name: test-local-srv
    project: security
    action: open
    allowed_domains:
      - 127.0.0.1
  - name: test-open-srv
    project: security
    action: open

global_deny_list:
  - stripe.com