git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request from GHSA-j7px-6hwj-hpjg
commitf5f13481761accdeaacc789fc5ba2be7df1b07ed authorJoel Speed <Joel.speed@hotmail.co.uk> dateWed 6 May 2020, 12:42 (6 years ago) parent4cdedc8
2 files changed+60−1
compare with master
unifiedsplit
oauthproxy.go+5−1 oauthproxy_test.go+55
oauthproxy.go+5−1
@@ -57,6 +57,10 @@ var SignatureHeaders = []string{
5757var (
5858 // ErrNeedsLogin means the user should be redirected to the login page
5959 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)?(/|\\)`)
6064)
6165
6266// OAuthProxy is the main authentication proxy
@@ -571,7 +575,7 @@ func validOptionalPort(port string) bool {
571575// IsValidRedirect checks whether the redirect URL is whitelisted
572576func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
573577 switch {
574− case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !strings.HasPrefix(redirect, "/\\"):
578+ case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
575579 return true
576580 case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
577581 redirectURL, err := url.Parse(redirect)
oauthproxy_test.go+55
@@ -323,6 +323,61 @@ func TestIsValidRedirect(t *testing.T) {
323323 Redirect: "http://a.sub.anyport.bar:8081/redirect",
324324 ExpectedResult: true,
325325 },
326+ {
327+ Desc: "openRedirect1",
328+ Redirect: "/\\evil.com",
329+ ExpectedResult: false,
330+ },
331+ {
332+ Desc: "openRedirectSpace1",
333+ Redirect: "/ /evil.com",
334+ ExpectedResult: false,
335+ },
336+ {
337+ Desc: "openRedirectSpace2",
338+ Redirect: "/ \\evil.com",
339+ ExpectedResult: false,
340+ },
341+ {
342+ Desc: "openRedirectTab1",
343+ Redirect: "/\t/evil.com",
344+ ExpectedResult: false,
345+ },
346+ {
347+ Desc: "openRedirectTab2",
348+ Redirect: "/\t\\evil.com",
349+ ExpectedResult: false,
350+ },
351+ {
352+ Desc: "openRedirectVerticalTab1",
353+ Redirect: "/\v/evil.com",
354+ ExpectedResult: false,
355+ },
356+ {
357+ Desc: "openRedirectVerticalTab2",
358+ Redirect: "/\v\\evil.com",
359+ ExpectedResult: false,
360+ },
361+ {
362+ Desc: "openRedirectNewLine1",
363+ Redirect: "/\n/evil.com",
364+ ExpectedResult: false,
365+ },
366+ {
367+ Desc: "openRedirectNewLine2",
368+ Redirect: "/\n\\evil.com",
369+ ExpectedResult: false,
370+ },
371+ {
372+ Desc: "openRedirectCarriageReturn1",
373+ Redirect: "/\r/evil.com",
374+ ExpectedResult: false,
375+ },
376+ {
377+ Desc: "openRedirectCarriageReturn2",
378+ Redirect: "/\r\\evil.com",
379+ ExpectedResult: false,
380+ },
326381 }
327382
328383 for _, tc := range testCases {