git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request from GHSA-j7px-6hwj-hpjg
0d5fa21Joel Speed
oauthproxy.go+5−1
compare with master
unifiedsplit
@@ -57,6 +57,10 @@ var SignatureHeaders = []string{
57var (
57var (
58 // ErrNeedsLogin means the user should be redirected to the login page
58 // ErrNeedsLogin means the user should be redirected to the login page
59 ErrNeedsLogin = errors.New("redirect to login page")
59 ErrNeedsLogin = errors.New("redirect to login page")
60+
61+ // Used to check final redirects are not susceptible to open redirects.
62+ // Matches //, /\ and both of these with whitespace in between (eg / / or / \).
63+ invalidRedirectRegex = regexp.MustCompile(`^/(\s|\v)?(/|\\)`)
60)
64)
61
65
62// OAuthProxy is the main authentication proxy
66// OAuthProxy is the main authentication proxy
@@ -578,7 +582,7 @@ func validOptionalPort(port string) bool {
578// IsValidRedirect checks whether the redirect URL is whitelisted
582// IsValidRedirect checks whether the redirect URL is whitelisted
579func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
583func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
580 switch {
584 switch {
581− case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !strings.HasPrefix(redirect, "/\\"):
585+ case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
582 return true
586 return true
583 case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
587 case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
584 redirectURL, err := url.Parse(redirect)
588 redirectURL, err := url.Parse(redirect)
oauthproxy_test.go+55
@@ -322,6 +322,61 @@ func TestIsValidRedirect(t *testing.T) {
322 Redirect: "http://a.sub.anyport.bar:8081/redirect",
322 Redirect: "http://a.sub.anyport.bar:8081/redirect",
323 ExpectedResult: true,
323 ExpectedResult: true,
324 },
324 },
325+ {
326+ Desc: "openRedirect1",
327+ Redirect: "/\\evil.com",
328+ ExpectedResult: false,
329+ },
330+ {
331+ Desc: "openRedirectSpace1",
332+ Redirect: "/ /evil.com",
333+ ExpectedResult: false,
334+ },
335+ {
336+ Desc: "openRedirectSpace2",
337+ Redirect: "/ \\evil.com",
338+ ExpectedResult: false,
339+ },
340+ {
341+ Desc: "openRedirectTab1",
342+ Redirect: "/\t/evil.com",
343+ ExpectedResult: false,
344+ },
345+ {
346+ Desc: "openRedirectTab2",
347+ Redirect: "/\t\\evil.com",
348+ ExpectedResult: false,
349+ },
350+ {
351+ Desc: "openRedirectVerticalTab1",
352+ Redirect: "/\v/evil.com",
353+ ExpectedResult: false,
354+ },
355+ {
356+ Desc: "openRedirectVerticalTab2",
357+ Redirect: "/\v\\evil.com",
358+ ExpectedResult: false,
359+ },
360+ {
361+ Desc: "openRedirectNewLine1",
362+ Redirect: "/\n/evil.com",
363+ ExpectedResult: false,
364+ },
365+ {
366+ Desc: "openRedirectNewLine2",
367+ Redirect: "/\n\\evil.com",
368+ ExpectedResult: false,
369+ },
370+ {
371+ Desc: "openRedirectCarriageReturn1",
372+ Redirect: "/\r/evil.com",
373+ ExpectedResult: false,
374+ },
375+ {
376+ Desc: "openRedirectCarriageReturn2",
377+ Redirect: "/\r\\evil.com",
378+ ExpectedResult: false,
379+ },
325 }
380 }
326
381
327 for _, tc := range testCases {
382 for _, tc := range testCases {