git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request from GHSA-j7px-6hwj-hpjg
commit0d5fa211df8ef2449347a56b22c779eb8d894c43 authorJoel Speed <Joel.speed@hotmail.co.uk> dateWed 6 May 2020, 12:42 (6 years ago) parent36da6e2
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
@@ -578,7 +582,7 @@ func validOptionalPort(port string) bool {
578582// IsValidRedirect checks whether the redirect URL is whitelisted
579583func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
580584 switch {
581− case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !strings.HasPrefix(redirect, "/\\"):
585+ case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
582586 return true
583587 case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
584588 redirectURL, err := url.Parse(redirect)
oauthproxy_test.go+55
@@ -322,6 +322,61 @@ func TestIsValidRedirect(t *testing.T) {
322322 Redirect: "http://a.sub.anyport.bar:8081/redirect",
323323 ExpectedResult: true,
324324 },
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+ },
325380 }
326381
327382 for _, tc := range testCases {