From cd22745e6ba488bf0e58b77eaf4db48438a8cb8d Mon Sep 17 00:00:00 2001
From: Ave <ave@ave.zone>
Date: Fri, 13 Jul 2018 07:31:33 +0300
Subject: [PATCH] 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.
---
 sanic/response.py      |  6 +++++-
 tests/test_redirect.py | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sanic/response.py b/sanic/response.py
index 62daf91e..2281766e 100644
--- a/sanic/response.py
+++ b/sanic/response.py
@@ -1,5 +1,6 @@
 from mimetypes import guess_type
 from os import path
+from urllib.parse import quote_plus
 
 try:
     from ujson import dumps as json_dumps
@@ -360,8 +361,11 @@ def redirect(to, headers=None, status=302,
     """
     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.
-    headers['Location'] = to
+    headers['Location'] = safe_to
 
     return HTTPResponse(
         status=status,
diff --git a/tests/test_redirect.py b/tests/test_redirect.py
index f5b734e3..5fdec2a6 100644
--- a/tests/test_redirect.py
+++ b/tests/test_redirect.py
@@ -32,6 +32,10 @@ def redirect_app():
     def handler(request):
         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
 
 
@@ -92,3 +96,16 @@ def test_chained_redirect(redirect_app):
         assert response.url.endswith('/3')
     except AttributeError:
         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')