From bf650a2ee78c6d1f4544a875dcc777cf27fe93e9 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Thu, 17 Jul 2014 21:59:28 +0200 Subject: [PATCH] [1.7.x] 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 8ca602c2414..3417ae3eab0 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -442,7 +442,11 @@ class RegexURLResolver(LocaleRegexProvider): candidate_pat = prefix_norm.replace('%', '%%') + result if re.search('^%s%s' % (prefix_norm, pattern), candidate_pat % candidate_subs, re.UNICODE): candidate_subs = dict((k, urlquote(v)) 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 6d197534e47..1ba822e7f66 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -151,6 +151,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 f7b160f6260..c4d10131be8 100644 --- a/tests/urlpatterns_reverse/urls.py +++ b/tests/urlpatterns_reverse/urls.py @@ -66,4 +66,7 @@ urlpatterns = patterns('', (r'defaults_view2/(?P\d+)/', 'defaults_view', {'arg2': 2}, 'defaults'), url('^includes/', include(other_patterns)), + + # Security tests + url('(.+)/security/$', empty_view, name='security'), )