From 5e5a17028f4b9cfb5ff777d8c259e079bca0c988 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Suliga?= Date: Fri, 19 Aug 2016 13:23:13 +0200 Subject: [PATCH] Fixed #26902 -- Allowed is_safe_url() to require an https URL. Thanks Andrew Nester, Berker Peksag, and Tim Graham for reviews. --- django/utils/http.py | 17 +++++++++++++---- tests/utils_tests/test_http.py | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/django/utils/http.py b/django/utils/http.py index 151e85de92..f662e395b4 100644 --- a/django/utils/http.py +++ b/django/utils/http.py @@ -277,12 +277,15 @@ def is_same_domain(host, pattern): ) -def is_safe_url(url, host=None): +def is_safe_url(url, host=None, require_https=False): """ Return ``True`` if the url is a safe redirection (i.e. it doesn't point to a different host and uses a safe scheme). Always returns ``False`` on an empty url. + + If ``require_https`` is ``True``, only 'https' will be considered a valid + scheme, as opposed to 'http' and 'https' with the default, ``False``. """ if url is not None: url = url.strip() @@ -295,10 +298,11 @@ def is_safe_url(url, host=None): return False # Chrome treats \ completely as / in paths but it could be part of some # basic auth credentials so we need to check both URLs. - return _is_safe_url(url, host) and _is_safe_url(url.replace('\\', '/'), host) + return (_is_safe_url(url, host, require_https=require_https) and + _is_safe_url(url.replace('\\', '/'), host, require_https=require_https)) -def _is_safe_url(url, host): +def _is_safe_url(url, host, require_https=False): # Chrome considers any URL with more than two slashes to be absolute, but # urlparse is not so flexible. Treat any url with three slashes as unsafe. if url.startswith('///'): @@ -315,8 +319,13 @@ def _is_safe_url(url, host): # URL and might consider the URL as scheme relative. if unicodedata.category(url[0])[0] == 'C': return False + scheme = url_info.scheme + # Consider URLs without a scheme (e.g. //example.com/p) to be http. + if not url_info.scheme and url_info.netloc: + scheme = 'http' + valid_schemes = ['https'] if require_https else ['http', 'https'] return ((not url_info.netloc or url_info.netloc == host) and - (not url_info.scheme or url_info.scheme in ['http', 'https'])) + (not scheme or scheme in valid_schemes)) def limited_parse_qsl(qs, keep_blank_values=False, encoding='utf-8', diff --git a/tests/utils_tests/test_http.py b/tests/utils_tests/test_http.py index e22f76be2e..b690055f30 100644 --- a/tests/utils_tests/test_http.py +++ b/tests/utils_tests/test_http.py @@ -140,6 +140,24 @@ class TestUtilsHttp(unittest.TestCase): # Basic auth without host is not allowed. self.assertFalse(http.is_safe_url(r'http://testserver\@example.com')) + def test_is_safe_url_secure_param_https_urls(self): + secure_urls = ( + 'https://example.com/p', + 'HTTPS://example.com/p', + '/view/?param=http://example.com', + ) + for url in secure_urls: + self.assertTrue(http.is_safe_url(url, 'example.com', require_https=True)) + + def test_is_safe_url_secure_param_non_https_urls(self): + not_secure_urls = ( + 'http://example.com/p', + 'ftp://example.com/p', + '//example.com/p', + ) + for url in not_secure_urls: + self.assertFalse(http.is_safe_url(url, 'example.com', require_https=True)) + def test_urlsafe_base64_roundtrip(self): bytestring = b'foo' encoded = http.urlsafe_base64_encode(bytestring)