CVE-2020-15234: Improper Handling of Case Sensitivity (CWE-178)
value: ORY Fosite is a security first OAuth2 & OpenID Connect framework for Go. In Fosite before version 0.34.1, the OAuth 2.0 Client's registered redirect URLs and the redirect URL provided at the OAuth2 Authorization Endpoint where compared using strings.ToLower while they should have been compared with a simple string match. This allows an attacker to register a client with allowed redirect URL https://example.com/callback. Then perform an OAuth2 flow and requesting redirect URL https://example.com/CALLBACK. Instead of an error (invalid redirect URL), the browser is redirected to https://example.com/CALLBACK with a potentially successful OAuth2 response, depending on the state of the overall OAuth2 flow (the user might still deny the request for example). This vulnerability has been patched in ORY Fosite v0.34.1.
CWE description: The product does not properly account for differences in case sensitivity when accessing or determining the properties of a resource, leading to inconsistent results.
CVSS v3 base score: 4.8
Published: 2020-10-02T21:15Z
Repository: https://github.com/ory/fosite
Language: Go
Files changed: authorize_helper.go, authorize_helper_test.go, helper_test.go
Commit date: 2020-10-02 15:15:49 +0200
Commit message: fix: make redirect URL checking more strict

The OAuth 2.0 Client's Redirect URL and the Redirect URL used in the OAuth 2.0 flow do not check if the query string is equal:

1. Registering a client with allowed redirect URL `https://example.com/callback`
2. Performing OAuth2 flow and requesting redirect URL `https://example.com/callback?bar=foo`
3. Instead of an error, the browser is redirected to `https://example.com/callback?bar=foo` with a potentially successful OAuth2 response.

Additionally, matching Redirect URLs used `strings.ToLower` normalization:

1. Registering a client with allowed redirect URL `https://example.com/callback`
2. Performing OAuth2 flow and requesting redirect URL `https://example.com/CALLBACK`
3. Instead of an error, the browser is redirected to `https://example.com/CALLBACK ` with a potentially successful OAuth2 response.

This patch addresses all of these issues and adds regression tests to keep the implementation secure in future releases.

Diff: diff --git a/authorize_helper.go b/authorize_helper.go
index ec286f58..68338fa4 100644
--- a/authorize_helper.go
+++ b/authorize_helper.go
@@ -116,28 +116,38 @@ func isMatchingRedirectURI(uri string, haystack []string) bool {
 	if err != nil {
 		return false
 	}
 
 	for _, b := range haystack {
-		if strings.ToLower(b) == strings.ToLower(uri) || isLoopbackURI(requested, b) {
+		if b == uri || isMatchingAsLoopback(requested, b) {
 			return true
 		}
 	}
 	return false
 }
 
-func isLoopbackURI(requested *url.URL, registeredURI string) bool {
+func isMatchingAsLoopback(requested *url.URL, registeredURI string) bool {
 	registered, err := url.Parse(registeredURI)
 	if err != nil {
 		return false
 	}
 
-	if registered.Scheme != "http" || !isLoopbackAddress(registered.Host) {
-		return false
-	}
-
-	if requested.Scheme == "http" && isLoopbackAddress(requested.Host) && registered.Path == requested.Path {
+	// Native apps that are able to open a port on the loopback network
+	// interface without needing special permissions (typically, those on
+	// desktop operating systems) can use the loopback interface to receive
+	// the OAuth redirect.
+	//
+	// Loopback redirect URIs use the "http" scheme and are constructed with
+	// the loopback IP literal and whatever port the client is listening on.
+	//
+	// Source: https://tools.ietf.org/html/rfc8252#section-7.3
+	if requested.Scheme == "http" &&
+		isLoopbackAddress(requested.Host) &&
+		registered.Hostname() == requested.Hostname() &&
+		// The port is skipped here - see codedoc above!
+		registered.Path == requested.Path &&
+		registered.RawQuery == requested.RawQuery {
 		return true
 	}
 
 	return false
 }
diff --git a/authorize_helper_test.go b/authorize_helper_test.go
index 77d31ede..77028465 100644
--- a/authorize_helper_test.go
+++ b/authorize_helper_test.go
@@ -135,77 +135,65 @@ func TestDoesClientWhiteListRedirect(t *testing.T) {
 			client:   &DefaultClient{RedirectURIs: []string{"https://bar.com/cb"}},
 			url:      "https://bar.com/cb",
 			isError:  false,
 			expected: "https://bar.com/cb",
 		},
-		{
-			client:   &DefaultClient{RedirectURIs: []string{"https://bar.Com/cb"}},
-			url:      "https://bar.com/cb",
-			isError:  false,
-			expected: "https://bar.com/cb",
-		},
-		{
-			client:   &DefaultClient{RedirectURIs: []string{"https://bar.com/cb"}},
-			url:      "https://bar.Com/cb",
-			isError:  false,
-			expected: "https://bar.Com/cb",
-		},
 		{
 			client:  &DefaultClient{RedirectURIs: []string{"https://bar.com/cb"}},
 			url:     "https://bar.com/cb123",
 			isError: true,
 		},
 		{
 			client:   &DefaultClient{RedirectURIs: []string{"http://[::1]"}},
 			url:      "http://[::1]:1024",
-			isError:  false,
 			expected: "http://[::1]:1024",
+			isError:  false,
 		},
 		{
 			client:  &DefaultClient{RedirectURIs: []string{"http://[::1]"}},
 			url:     "http://[::1]:1024/cb",
 			isError: true,
 		},
 		{
 			client:   &DefaultClient{RedirectURIs: []string{"http://[::1]/cb"}},
 			url:      "http://[::1]:1024/cb",
-			isError:  false,
 			expected: "http://[::1]:1024/cb",
+			isError:  false,
 		},
 		{
 			client:  &DefaultClient{RedirectURIs: []string{"http://[::1]"}},
 			url:     "http://foo.bar/bar",
 			isError: true,
 		},
 		{
 			client:   &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
 			url:      "http://127.0.0.1:1024",
-			isError:  false,
 			expected: "http://127.0.0.1:1024",
+			isError:  false,
 		},
 		{
 			client:   &DefaultClient{RedirectURIs: []string{"http://127.0.0.1/cb"}},
 			url:      "http://127.0.0.1:64000/cb",
-			isError:  false,
 			expected: "http://127.0.0.1:64000/cb",
+			isError:  false,
 		},
 		{
 			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
 			url:     "http://127.0.0.1:64000/cb",
 			isError: true,
 		},
 		{
 			client:   &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
 			url:      "http://127.0.0.1",
-			isError:  false,
 			expected: "http://127.0.0.1",
+			isError:  false,
 		},
 		{
 			client:   &DefaultClient{RedirectURIs: []string{"http://127.0.0.1/Cb"}},
 			url:      "http://127.0.0.1:8080/Cb",
-			isError:  false,
 			expected: "http://127.0.0.1:8080/Cb",
+			isError:  false,
 		},
 		{
 			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
 			url:     "http://foo.bar/bar",
 			isError: true,
@@ -213,13 +201,49 @@ func TestDoesClientWhiteListRedirect(t *testing.T) {
 		{
 			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1"}},
 			url:     ":/invalid.uri)bar",
 			isError: true,
 		},
+		{
+			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb"}},
+			url:     "http://127.0.0.1:8080/Cb",
+			isError: true,
+		},
+		{
+			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb"}},
+			url:     "http://127.0.0.1:8080/cb?foo=bar",
+			isError: true,
+		},
+		{
+			client:   &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb?foo=bar"}},
+			url:      "http://127.0.0.1:8080/cb?foo=bar",
+			expected: "http://127.0.0.1:8080/cb?foo=bar",
+			isError:  false,
+		},
+		{
+			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb?foo=bar"}},
+			url:     "http://127.0.0.1:8080/cb?baz=bar&foo=bar",
+			isError: true,
+		},
+		{
+			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb?foo=bar&baz=bar"}},
+			url:     "http://127.0.0.1:8080/cb?baz=bar&foo=bar",
+			isError: true,
+		},
+		{
+			client:  &DefaultClient{RedirectURIs: []string{"https://www.ory.sh/cb"}},
+			url:     "http://127.0.0.1:8080/cb",
+			isError: true,
+		},
+		{
+			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb"}},
+			url:     "https://www.ory.sh/cb",
+			isError: true,
+		},
 	} {
 		redir, err := MatchRedirectURIWithClientRedirectURIs(c.url, c.client)
-		assert.Equal(t, c.isError, err != nil, "%d: %s", k, err)
+		assert.Equal(t, c.isError, err != nil, "%d: %+v", k, c)
 		if err == nil {
 			require.NotNil(t, redir, "%d", k)
 			assert.Equal(t, c.expected, redir.String(), "%d", k)
 		}
 		t.Logf("Passed test case %d", k)
diff --git a/helper_test.go b/helper_test.go
index dd4000b6..a7952c1d 100644
--- a/helper_test.go
+++ b/helper_test.go
@@ -49,10 +49,10 @@ func TestStringInSlice(t *testing.T) {
 
 func TestEscapeJSONString(t *testing.T) {
 	for _, str := range []string{"", "foobar", `foo"bar`, `foo\bar`, "foo\n\tbar"} {
 		escaped := EscapeJSONString(str)
 		var unmarshaled string
-		err := json.Unmarshal([]byte(`"` + escaped + `"`), &unmarshaled)
+		err := json.Unmarshal([]byte(`"`+escaped+`"`), &unmarshaled)
 		require.NoError(t, err, str)
 		assert.Equal(t, str, unmarshaled, str)
 	}
 }

Vulnerable code: 		if strings.ToLower(b) == strings.ToLower(uri) || isLoopbackURI(requested, b) {
func isLoopbackURI(requested *url.URL, registeredURI string) bool {
	if registered.Scheme != "http" || !isLoopbackAddress(registered.Host) {
		return false
	}

	if requested.Scheme == "http" && isLoopbackAddress(requested.Host) && registered.Path == requested.Path {
		{
			client:   &DefaultClient{RedirectURIs: []string{"https://bar.Com/cb"}},
			url:      "https://bar.com/cb",
			isError:  false,
			expected: "https://bar.com/cb",
		},
		{
			client:   &DefaultClient{RedirectURIs: []string{"https://bar.com/cb"}},
			url:      "https://bar.Com/cb",
			isError:  false,
			expected: "https://bar.Com/cb",
		},
			isError:  false,
			isError:  false,
			isError:  false,
			isError:  false,
			isError:  false,
			isError:  false,
		assert.Equal(t, c.isError, err != nil, "%d: %s", k, err)
		err := json.Unmarshal([]byte(`"` + escaped + `"`), &unmarshaled)
Fixed code: 		if b == uri || isMatchingAsLoopback(requested, b) {
func isMatchingAsLoopback(requested *url.URL, registeredURI string) bool {
	// Native apps that are able to open a port on the loopback network
	// interface without needing special permissions (typically, those on
	// desktop operating systems) can use the loopback interface to receive
	// the OAuth redirect.
	//
	// Loopback redirect URIs use the "http" scheme and are constructed with
	// the loopback IP literal and whatever port the client is listening on.
	//
	// Source: https://tools.ietf.org/html/rfc8252#section-7.3
	if requested.Scheme == "http" &&
		isLoopbackAddress(requested.Host) &&
		registered.Hostname() == requested.Hostname() &&
		// The port is skipped here - see codedoc above!
		registered.Path == requested.Path &&
		registered.RawQuery == requested.RawQuery {
			isError:  false,
			isError:  false,
			isError:  false,
			isError:  false,
			isError:  false,
			isError:  false,
		{
			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb"}},
			url:     "http://127.0.0.1:8080/Cb",
			isError: true,
		},
		{
			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb"}},
			url:     "http://127.0.0.1:8080/cb?foo=bar",
			isError: true,
		},
		{
			client:   &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb?foo=bar"}},
			url:      "http://127.0.0.1:8080/cb?foo=bar",
			expected: "http://127.0.0.1:8080/cb?foo=bar",
			isError:  false,
		},
		{
			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb?foo=bar"}},
			url:     "http://127.0.0.1:8080/cb?baz=bar&foo=bar",
			isError: true,
		},
		{
			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb?foo=bar&baz=bar"}},
			url:     "http://127.0.0.1:8080/cb?baz=bar&foo=bar",
			isError: true,
		},
		{
			client:  &DefaultClient{RedirectURIs: []string{"https://www.ory.sh/cb"}},
			url:     "http://127.0.0.1:8080/cb",
			isError: true,
		},
		{
			client:  &DefaultClient{RedirectURIs: []string{"http://127.0.0.1:8080/cb"}},
			url:     "https://www.ory.sh/cb",
			isError: true,
		},
		assert.Equal(t, c.isError, err != nil, "%d: %+v", k, c)
		err := json.Unmarshal([]byte(`"`+escaped+`"`), &unmarshaled)