Sanitize the URL before redirecting (#1260)

* URL Quote the URL before redirecting

* Use safe url instead of unsafe one

* Fix query params

* fix build

* Whitelist all reserved characters from rfc3986

* Add tests for redirect url sanitizing

* Remove check for resulting URL on header injection test

The thing the tests are testing for can be implemented in other
ways that don't redirect to 100% the same address, but they'll all have
to match the remaining parts of the test to succeed.
This commit is contained in:
Ave 2018-07-13 07:31:33 +03:00 committed by Raphael Deem
parent 334649dfd4
commit cd22745e6b
2 changed files with 22 additions and 1 deletions

View File

@ -1,5 +1,6 @@
from mimetypes import guess_type from mimetypes import guess_type
from os import path from os import path
from urllib.parse import quote_plus
try: try:
from ujson import dumps as json_dumps from ujson import dumps as json_dumps
@ -360,8 +361,11 @@ def redirect(to, headers=None, status=302,
""" """
headers = headers or {} headers = headers or {}
# URL Quote the URL before redirecting
safe_to = quote_plus(to, safe=":/#?&=@[]!$&'()*+,;")
# According to RFC 7231, a relative URI is now permitted. # According to RFC 7231, a relative URI is now permitted.
headers['Location'] = to headers['Location'] = safe_to
return HTTPResponse( return HTTPResponse(
status=status, status=status,

View File

@ -32,6 +32,10 @@ def redirect_app():
def handler(request): def handler(request):
return text('OK') return text('OK')
@app.route('/redirect_with_header_injection')
async def redirect_with_header_injection(request):
return redirect("/unsafe\ntest-header: test-value\n\ntest-body")
return app return app
@ -92,3 +96,16 @@ def test_chained_redirect(redirect_app):
assert response.url.endswith('/3') assert response.url.endswith('/3')
except AttributeError: except AttributeError:
assert response.url.path.endswith('/3') assert response.url.path.endswith('/3')
def test_redirect_with_header_injection(redirect_app):
"""
Test redirection to a URL with header and body injections.
"""
request, response = redirect_app.test_client.get(
"/redirect_with_header_injection",
allow_redirects=False)
assert response.status == 302
assert "test-header" not in response.headers
assert not response.text.startswith('test-body')