Open redirect possibility with newer werkzeug

[jwag956]

With werkzeug >=2.1 the autocorrect_location_header configuration was changed to False - which means that location headers in redirects are relative by default. This opens up a possible open-redirect since (all?) browsers are known to take many ill-formed URLs which would be 'relative' in spec standards and convert them (by removing backslashes etc) to absolute URLs.

This issue:https://github.com/pallets/werkzeug/issues/2821 asks werkzeug to help - though of course this isn't actually a security issue for them (or flask).

This CVE: https://www.cve.org/CVERecord?id=CVE-2021-32618 covers the original concern (and mitigation).

This latest vulnerability due to a change in Werkzeug 2.1 is covered in: CVE-2023-49438

Finally user of Flask-Security found additional cases that would trigger this:

Here are the two cases I've found to still work which can be added to test_misc.py within the test_validate_redirect function:

assert not validate_redirect_url("/\github.com")
assert not validate_redirect_url("\/github.com")

Also,

This requires further validation but modifying "REDIRECT_VALIDATE_RE" (line 201 of core.py in the latest release) to the following regex seems to patch the issue in my testing thus far:

r"^/{4,}|\\{3,}|[\s\000-\037][/\\]{2,}(?![/\\])|[/\\]([^/\\]|/[^/\\])*[/\\].*"

Since this is not the default behavior of werkzeug we need to do something in FS to mitigate it. Ideas:

• Change the behavior of autocorrect_location_header to True
• continue to improve the regex.
• add config variable to ignore 'next' parameter (this likely has a lot of backwards compat issues

[gmanfunky]

@jwag956 I think much of the validate_redirect_url() concerns identified would be mitigated by encoding the URL parts before using them with redirect().

Unencoded backslashes, for example, are invalid relative path characters per rfc3986.html#section-2.3

More explicit attention to redirect URL component encoding could be prudent considering next= is delivered as a string via URL query parameter and may need to be decoded, parsed, and re-encoded for different contexts (i.e., relative URL + query parameters + ??)

For example,

from urllib.parse import quote, urlsplit, urlunsplit
invalid_relative_url = '/\\\\github.com?extra=data&blank=#fragment'

(scheme, netloc, path, query, fragment) = urlsplit(invalid_relative_url)
# SplitResult(scheme='', netloc='', path='/\\\\github.com', query='extra=data&blank=', fragment='fragment')

definitely_relative_url_for_browsers = urlunsplit(( "", "", quote(path), query, fragment))
# '/%5C%5Cgithub.com?extra=data&blank=#fragment'

# Now it's safe to send to Flask using `redirect(definitely_relative_url_for_browsers)``

Overall, it appears Python doesn't have a standard canonical URL data class. urllib.parse methods are limited to making a general effort parse something out of a string without validation. Perhaps Pydantic could fill this gap if we really need to get into validation.

[jwag956]

Thanks - great ideas/thoughts here. That is basically what Werkzeugs 'autocorrect_location_header' configuration does - which is why, when that was the default, this issue didn't arise. But I like the idea of taking this into Flask-Security and getting rid of the RE mess.