Merge pull request from GHSA-j7px-6hwj-hpjg

oauthproxy.go

@@ -57,6 +57,10 @@ var SignatureHeaders = []string{
 var (
 	// ErrNeedsLogin means the user should be redirected to the login page
 	ErrNeedsLogin = errors.New("redirect to login page")
+
+	// Used to check final redirects are not susceptible to open redirects.
+	// Matches //, /\ and both of these with whitespace in between (eg / / or / \).
+	invalidRedirectRegex = regexp.MustCompile(`^/(\s|\v)?(/|\\)`)
 )
 
 // OAuthProxy is the main authentication proxy
@@ -571,7 +575,7 @@ func validOptionalPort(port string) bool {
 // IsValidRedirect checks whether the redirect URL is whitelisted
 func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
 	switch {
-	case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !strings.HasPrefix(redirect, "/\\"):
+	case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
 		return true
 	case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
 		redirectURL, err := url.Parse(redirect)

oauthproxy_test.go

@@ -323,6 +323,61 @@ func TestIsValidRedirect(t *testing.T) {
 			Redirect:       "http://a.sub.anyport.bar:8081/redirect",
 			ExpectedResult: true,
 		},
+		{
+			Desc:           "openRedirect1",
+			Redirect:       "/\\evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectSpace1",
+			Redirect:       "/ /evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectSpace2",
+			Redirect:       "/ \\evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectTab1",
+			Redirect:       "/\t/evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectTab2",
+			Redirect:       "/\t\\evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectVerticalTab1",
+			Redirect:       "/\v/evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectVerticalTab2",
+			Redirect:       "/\v\\evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectNewLine1",
+			Redirect:       "/\n/evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectNewLine2",
+			Redirect:       "/\n\\evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectCarriageReturn1",
+			Redirect:       "/\r/evil.com",
+			ExpectedResult: false,
+		},
+		{
+			Desc:           "openRedirectCarriageReturn2",
+			Redirect:       "/\r\\evil.com",
+			ExpectedResult: false,
+		},
 	}
 
 	for _, tc := range testCases {