From 28e765810df46a3f28ff4785491e9973593382fd Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Thu, 17 Jul 2014 21:59:28 +0200 Subject: [PATCH] Prevented reverse() from generating URLs pointing to other hosts. This is a security fix. Disclosure following shortly. --- django/core/urlresolvers.py | 6 +++++- docs/releases/1.4.14.txt | 13 +++++++++++++ docs/releases/1.5.9.txt | 13 +++++++++++++ docs/releases/1.6.6.txt | 13 +++++++++++++ tests/urlpatterns_reverse/tests.py | 3 +++ tests/urlpatterns_reverse/urls.py | 3 +++ 6 files changed, 50 insertions(+), 1 deletion(-) diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index d3bfef423bb..9a399466bcb 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -456,7 +456,11 @@ class RegexURLResolver(LocaleRegexProvider): # safe characters from `pchar` definition of RFC 3986 candidate_subs = dict((k, urlquote(v, safe=RFC3986_SUBDELIMS + str('/~:@'))) for (k, v) in candidate_subs.items()) - return candidate_pat % candidate_subs + url = candidate_pat % candidate_subs + # Don't allow construction of scheme relative urls. + if url.startswith('//'): + url = '/%%2F%s' % url[2:] + return url # lookup_view can be URL label, or dotted path, or callable, Any of # these can be passed in at the top, but callables are not friendly in # error messages. diff --git a/docs/releases/1.4.14.txt b/docs/releases/1.4.14.txt index d0032e53990..28390c96a40 100644 --- a/docs/releases/1.4.14.txt +++ b/docs/releases/1.4.14.txt @@ -5,3 +5,16 @@ Django 1.4.14 release notes *Under development* Django 1.4.14 fixes several security issues in 1.4.13. + +:func:`~django.core.urlresolvers.reverse()` could generate URLs pointing to other hosts +======================================================================================= + +In certain situations, URL reversing could generate scheme-relative URLs (URLs +starting with two slashes), which could unexpectedly redirect a user to a +different host. An attacker could exploit this, for example, by redirecting +users to a phishing site designed to ask for user's passwords. + +To remedy this, URL reversing now ensures that no URL starts with two slashes +(//), replacing the second slash with its URL encoded counterpart (%2F). This +approach ensures that semantics stay the same, while making the URL relative to +the domain and not to the scheme. diff --git a/docs/releases/1.5.9.txt b/docs/releases/1.5.9.txt index 4a5233a1fdf..12b5b5f8062 100644 --- a/docs/releases/1.5.9.txt +++ b/docs/releases/1.5.9.txt @@ -5,3 +5,16 @@ Django 1.5.9 release notes *Under development* Django 1.5.9 fixes several security issues in 1.5.8. + +:func:`~django.core.urlresolvers.reverse()` could generate URLs pointing to other hosts +======================================================================================= + +In certain situations, URL reversing could generate scheme-relative URLs (URLs +starting with two slashes), which could unexpectedly redirect a user to a +different host. An attacker could exploit this, for example, by redirecting +users to a phishing site designed to ask for user's passwords. + +To remedy this, URL reversing now ensures that no URL starts with two slashes +(//), replacing the second slash with its URL encoded counterpart (%2F). This +approach ensures that semantics stay the same, while making the URL relative to +the domain and not to the scheme. diff --git a/docs/releases/1.6.6.txt b/docs/releases/1.6.6.txt index 0a4519b04d2..43b3adf630b 100644 --- a/docs/releases/1.6.6.txt +++ b/docs/releases/1.6.6.txt @@ -6,6 +6,19 @@ Django 1.6.6 release notes Django 1.6.6 fixes several security issues and bugs in 1.6.5. +:func:`~django.core.urlresolvers.reverse()` could generate URLs pointing to other hosts +======================================================================================= + +In certain situations, URL reversing could generate scheme-relative URLs (URLs +starting with two slashes), which could unexpectedly redirect a user to a +different host. An attacker could exploit this, for example, by redirecting +users to a phishing site designed to ask for user's passwords. + +To remedy this, URL reversing now ensures that no URL starts with two slashes +(//), replacing the second slash with its URL encoded counterpart (%2F). This +approach ensures that semantics stay the same, while making the URL relative to +the domain and not to the scheme. + Bugfixes ======== diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 5603ea5f154..dbb1836b7a7 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -152,6 +152,9 @@ test_data = ( ('defaults', '/defaults_view2/3/', [], {'arg1': 3, 'arg2': 2}), ('defaults', NoReverseMatch, [], {'arg1': 3, 'arg2': 3}), ('defaults', NoReverseMatch, [], {'arg2': 1}), + + # Security tests + ('security', '/%2Fexample.com/security/', ['/example.com'], {}), ) diff --git a/tests/urlpatterns_reverse/urls.py b/tests/urlpatterns_reverse/urls.py index 943f92e9569..4ba596d81c7 100644 --- a/tests/urlpatterns_reverse/urls.py +++ b/tests/urlpatterns_reverse/urls.py @@ -75,4 +75,7 @@ with warnings.catch_warnings(record=True): (r'defaults_view2/(?P[0-9]+)/', defaults_view, {'arg2': 2}, 'defaults'), url('^includes/', include(other_patterns)), + + # Security tests + url('(.+)/security/$', empty_view, name='security'), )