1
0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2025-03-19 21:27:58 +02:00

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

This commit is contained in:
Joel Speed 2020-05-06 12:42:02 +01:00 committed by GitHub
parent 36da6e2be9
commit 0d5fa211df
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 1 deletions

View File

@ -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
@ -578,7 +582,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)

View File

@ -322,6 +322,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 {