handlers: reject protocol-relative URLs in redirect validation#5281
handlers: reject protocol-relative URLs in redirect validation#5281adilburaksen wants to merge 2 commits into
Conversation
_SAFE_URL_PATTERN's relative-URL branch matched the empty string prefix of '//evil.com', so check_redirect_url accepted protocol-relative URLs as safe. Browsers interpret '//evil.com/path' as 'same-scheme://evil.com/path', enabling open redirect from any endpoint that passes the user-supplied 'next' / 'redirect' parameter through check_redirect_url. Add a negative lookahead (?!//) before the relative-URL branch so that any URL beginning with '//' is rejected unless it already matched the explicit-scheme branch (http://, https://, etc).
|
Gentle ping after ~18 days — small change rejecting protocol-relative URLs in the redirect handler to close an open-redirect. CI is green; glad to add a test case or adjust if you'd like a tighter check. |
|
Pushed a follow-up that closes a gap in the original The scheme-relative guard alone is bypassable, because browsers normalize backslashes to forward slashes and strip ASCII tab/newline and leading control/whitespace before navigating. Since
Both The added pre-check rejects any input with a backslash, a control character, or leading/trailing whitespace before the pattern match. Verified it blocks all of the above plus the original |
Summary
check_redirect_urlinsrc/appengine/handlers/base_handler.pyuses_SAFE_URL_PATTERN(based on the Closure LibrarySafeUrlpattern) tovalidate redirect targets. The relative-URL branch of that pattern is:
[^:/?#]*can match zero characters, after which(?:[/?#]|$)matchesthe leading
/of//evil.com. This means the pattern accepts anyprotocol-relative URL such as
//attacker.example.com/stealas safe.Browsers interpret
//evil.com/pathas scheme-relative (same scheme asthe current page), so a
next=//evil.com/loginparameter causes apost-login redirect to the attacker's host.
Fix
Add a negative lookahead
(?!//)to the relative-URL branch:This rejects any URL that the relative branch would match but that begins
with
//, while continuing to accept ordinary relative paths (/login,relative/path) and absolute URLs with safe schemes.Verification