p15693087
/
django
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Refs #26956 -- Allowed is_safe_url() to validate against multiple hosts

This commit is contained in:
Jon Dufresne 2016-07-26 20:45:07 -07:00
parent 978a00e39f
commit f227b8d15d
6 changed files with 48 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
django/contrib/auth/views.py
View File

@ -86,7 +86,7 @@ class LoginView(FormView):
)
url_is_safe = is_safe_url(
url=redirect_to,
host=self.request.get_host(),
allowed_hosts={self.request.get_host()},
require_https=self.request.is_secure(),
)
if not url_is_safe:
@ -157,7 +157,7 @@ class LogoutView(TemplateView):
)
url_is_safe = is_safe_url(
url=next_page,
host=self.request.get_host(),
allowed_hosts={self.request.get_host()},
require_https=self.request.is_secure(),
)
# Security check -- don't allow redirection to a different host.

22
django/utils/http.py
View File

@ -6,12 +6,14 @@ import datetime
import re
import sys
import unicodedata
import warnings
from binascii import Error as BinasciiError
from email.utils import formatdate
from django.core.exceptions import TooManyFieldsSent
from django.utils import six
from django.utils.datastructures import MultiValueDict
from django.utils.deprecation import RemovedInDjango21Warning
from django.utils.encoding import force_bytes, force_str, force_text
from django.utils.functional import keep_lazy_text
from django.utils.six.moves.urllib.parse import (
@ -277,7 +279,7 @@ def is_same_domain(host, pattern):
)
def is_safe_url(url, host=None, require_https=False):
def is_safe_url(url, host=None, allowed_hosts=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).
@ -296,13 +298,23 @@ def is_safe_url(url, host=None, require_https=False):
url = force_text(url)
except UnicodeDecodeError:
return False
if allowed_hosts is None:
allowed_hosts = set()
if host:
warnings.warn(
"The host argument is deprecated, use allowed_hosts instead.",
RemovedInDjango21Warning,
stacklevel=2,
)
# Avoid mutating the passed in allowed_hosts.
allowed_hosts = allowed_hosts | {host}
# 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, require_https=require_https) and
_is_safe_url(url.replace('\\', '/'), host, require_https=require_https))
return (_is_safe_url(url, allowed_hosts, require_https=require_https) and
_is_safe_url(url.replace('\\', '/'), allowed_hosts, require_https=require_https))
def _is_safe_url(url, host, require_https=False):
def _is_safe_url(url, allowed_hosts, 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('///'):
@ -324,7 +336,7 @@ def _is_safe_url(url, host, require_https=False):
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
return ((not url_info.netloc or url_info.netloc in allowed_hosts) and
(not scheme or scheme in valid_schemes))

4
django/views/i18n.py
View File

@ -38,11 +38,11 @@ def set_language(request):
"""
next = request.POST.get('next', request.GET.get('next'))
if ((next or not request.is_ajax()) and
not is_safe_url(url=next, host=request.get_host(), require_https=request.is_secure())):
not is_safe_url(url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure())):
next = request.META.get('HTTP_REFERER')
if next:
next = urlunquote(next) # HTTP_REFERER may be encoded.
if not is_safe_url(url=next, host=request.get_host(), require_https=request.is_secure()):
if not is_safe_url(url=next, allowed_hosts={request.get_host()}, require_https=request.is_secure()):
next = '/'
response = http.HttpResponseRedirect(next) if next else http.HttpResponse(status=204)
if request.method == 'POST':

3
docs/internals/deprecation.txt
View File

@ -30,6 +30,9 @@ details on these changes.
* ``django.core.cache.backends.memcached.PyLibMCCache`` will no longer support
passing ``pylibmc`` behavior settings as top-level attributes of ``OPTIONS``.
* The ``host`` parameter of ``django.utils.http.is_safe_url()`` will be
removed.
.. _deprecation-removed-in-2.0:
2.0

3
docs/releases/1.11.txt
View File

@ -520,3 +520,6 @@ Miscellaneous
* For the ``PyLibMCCache`` cache backend, passing ``pylibmc`` behavior settings
as top-level attributes of ``OPTIONS`` is deprecated. Set them under a
``behaviors`` key within ``OPTIONS`` instead.
* The ``host`` parameter of ``django.utils.http.is_safe_url()`` is deprecated
in favor of the new ``allowed_hosts`` parameter.

30
tests/utils_tests/test_http.py
View File

@ -5,8 +5,10 @@ import sys
import unittest
from datetime import datetime
from django.test import ignore_warnings
from django.utils import http, six
from django.utils.datastructures import MultiValueDict
from django.utils.deprecation import RemovedInDjango21Warning
class TestUtilsHttp(unittest.TestCase):
@ -107,7 +109,12 @@ class TestUtilsHttp(unittest.TestCase):
'\n',
)
for bad_url in bad_urls:
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
with ignore_warnings(category=RemovedInDjango21Warning):
self.assertFalse(http.is_safe_url(bad_url, host='testserver'), "%s should be blocked" % bad_url)
self.assertFalse(
http.is_safe_url(bad_url, allowed_hosts={'testserver', 'testserver2'}),
"%s should be blocked" % bad_url,
)
good_urls = (
'/view/?param=http://example.com',
@ -121,20 +128,25 @@ class TestUtilsHttp(unittest.TestCase):
'/url%20with%20spaces/',
)
for good_url in good_urls:
self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
with ignore_warnings(category=RemovedInDjango21Warning):
self.assertTrue(http.is_safe_url(good_url, host='testserver'), "%s should be allowed" % good_url)
self.assertTrue(
http.is_safe_url(good_url, allowed_hosts={'otherserver', 'testserver'}),
"%s should be allowed" % good_url,
)
if six.PY2:
# Check binary URLs, regression tests for #26308
self.assertTrue(
http.is_safe_url(b'https://testserver/', host='testserver'),
http.is_safe_url(b'https://testserver/', allowed_hosts={'testserver'}),
"binary URLs should be allowed on Python 2"
)
self.assertFalse(http.is_safe_url(b'\x08//example.com', host='testserver'))
self.assertTrue(http.is_safe_url('àview/'.encode('utf-8'), host='testserver'))
self.assertFalse(http.is_safe_url('àview'.encode('latin-1'), host='testserver'))
self.assertFalse(http.is_safe_url(b'\x08//example.com', allowed_hosts={'testserver'}))
self.assertTrue(http.is_safe_url('àview/'.encode('utf-8'), allowed_hosts={'testserver'}))
self.assertFalse(http.is_safe_url('àview'.encode('latin-1'), allowed_hosts={'testserver'}))
# Valid basic auth credentials are allowed.
self.assertTrue(http.is_safe_url(r'http://user:pass@testserver/', host='user:pass@testserver'))
self.assertTrue(http.is_safe_url(r'http://user:pass@testserver/', allowed_hosts={'user:pass@testserver'}))
# A path without host is allowed.
self.assertTrue(http.is_safe_url('/confirm/me@example.com'))
# Basic auth without host is not allowed.
@ -147,7 +159,7 @@ class TestUtilsHttp(unittest.TestCase):
'/view/?param=http://example.com',
)
for url in secure_urls:
self.assertTrue(http.is_safe_url(url, 'example.com', require_https=True))
self.assertTrue(http.is_safe_url(url, allowed_hosts={'example.com'}, require_https=True))
def test_is_safe_url_secure_param_non_https_urls(self):
not_secure_urls = (
@ -156,7 +168,7 @@ class TestUtilsHttp(unittest.TestCase):
'//example.com/p',
)
for url in not_secure_urls:
self.assertFalse(http.is_safe_url(url, 'example.com', require_https=True))
self.assertFalse(http.is_safe_url(url, allowed_hosts={'example.com'}, require_https=True))
def test_urlsafe_base64_roundtrip(self):
bytestring = b'foo'