summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTim Graham <timograham@gmail.com>2014-04-20 13:29:12 -0400
committerTim Graham <timograham@gmail.com>2014-04-21 18:29:12 -0400
commit546740544d7f69254a67b06a3fc7fa0c43512958 (patch)
treea7a3133de74dfff0bcfbb3d21c524198d33a3303
parent0bd913a19cf0e7515834d6cb6d9e668840fecce5 (diff)
[1.7.x] Fixed a remote code execution vulnerabilty in URL reversing.
Thanks Benjamin Bach for the report and initial patch. This is a security fix; disclosure to follow shortly. Backport of 8b93b31487d6d3b0fcbbd0498991ea0db9088054 from master
-rw-r--r--django/core/urlresolvers.py21
-rw-r--r--tests/urlpatterns_reverse/nonimported_module.py3
-rw-r--r--tests/urlpatterns_reverse/tests.py21
-rw-r--r--tests/urlpatterns_reverse/urls.py1
-rw-r--r--tests/urlpatterns_reverse/views.py4
5 files changed, 49 insertions, 1 deletions
diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py
index 0454b5f366..eebe54e99b 100644
--- a/django/core/urlresolvers.py
+++ b/django/core/urlresolvers.py
@@ -245,6 +245,10 @@ class RegexURLResolver(LocaleRegexProvider):
self._reverse_dict = {}
self._namespace_dict = {}
self._app_dict = {}
+ # set of dotted paths to all functions and classes that are used in
+ # urlpatterns
+ self._callback_strs = set()
+ self._populated = False
def __repr__(self):
if isinstance(self.urlconf_name, list) and len(self.urlconf_name):
@@ -262,6 +266,15 @@ class RegexURLResolver(LocaleRegexProvider):
apps = {}
language_code = get_language()
for pattern in reversed(self.url_patterns):
+ if hasattr(pattern, '_callback_str'):
+ self._callback_strs.add(pattern._callback_str)
+ elif hasattr(pattern, '_callback'):
+ callback = pattern._callback
+ if not hasattr(callback, '__name__'):
+ lookup_str = callback.__module__ + "." + callback.__class__.__name__
+ else:
+ lookup_str = callback.__module__ + "." + callback.__name__
+ self._callback_strs.add(lookup_str)
p_pattern = pattern.regex.pattern
if p_pattern.startswith('^'):
p_pattern = p_pattern[1:]
@@ -280,6 +293,7 @@ class RegexURLResolver(LocaleRegexProvider):
namespaces[namespace] = (p_pattern + prefix, sub_pattern)
for app_name, namespace_list in pattern.app_dict.items():
apps.setdefault(app_name, []).extend(namespace_list)
+ self._callback_strs.update(pattern._callback_strs)
else:
bits = normalize(p_pattern)
lookups.appendlist(pattern.callback, (bits, p_pattern, pattern.default_args))
@@ -288,6 +302,7 @@ class RegexURLResolver(LocaleRegexProvider):
self._reverse_dict[language_code] = lookups
self._namespace_dict[language_code] = namespaces
self._app_dict[language_code] = apps
+ self._populated = True
@property
def reverse_dict(self):
@@ -387,8 +402,12 @@ class RegexURLResolver(LocaleRegexProvider):
text_args = [force_text(v) for v in args]
text_kwargs = dict((k, force_text(v)) for (k, v) in kwargs.items())
+ if not self._populated:
+ self._populate()
+
try:
- lookup_view = get_callable(lookup_view, True)
+ if lookup_view in self._callback_strs:
+ lookup_view = get_callable(lookup_view, True)
except (ImportError, AttributeError) as e:
raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e))
possibilities = self.reverse_dict.getlist(lookup_view)
diff --git a/tests/urlpatterns_reverse/nonimported_module.py b/tests/urlpatterns_reverse/nonimported_module.py
new file mode 100644
index 0000000000..df046333d3
--- /dev/null
+++ b/tests/urlpatterns_reverse/nonimported_module.py
@@ -0,0 +1,3 @@
+def view(request):
+ """Stub view"""
+ pass
diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py
index a57b7e2a17..b2b2d5fc65 100644
--- a/tests/urlpatterns_reverse/tests.py
+++ b/tests/urlpatterns_reverse/tests.py
@@ -1,8 +1,10 @@
+# -*- coding: utf-8 -*-
"""
Unit tests for reverse URL lookups.
"""
from __future__ import unicode_literals
+import sys
import unittest
from django.contrib.auth.models import User
@@ -355,6 +357,25 @@ class ReverseShortcutTests(TestCase):
self.assertEqual(res.url, '/foo/')
res = redirect('http://example.com/')
self.assertEqual(res.url, 'http://example.com/')
+ # Assert that we can redirect using UTF-8 strings
+ res = redirect('/æøå/abc/')
+ self.assertEqual(res.url, '/%C3%A6%C3%B8%C3%A5/abc/')
+ # Assert that no imports are attempted when dealing with a relative path
+ # (previously, the below would resolve in a UnicodeEncodeError from __import__ )
+ res = redirect('/æøå.abc/')
+ self.assertEqual(res.url, '/%C3%A6%C3%B8%C3%A5.abc/')
+ res = redirect('os.path')
+ self.assertEqual(res.url, 'os.path')
+
+ def test_no_illegal_imports(self):
+ # modules that are not listed in urlpatterns should not be importable
+ redirect("urlpatterns_reverse.nonimported_module.view")
+ self.assertNotIn("urlpatterns_reverse.nonimported_module", sys.modules)
+
+ def test_reverse_by_path_nested(self):
+ # Views that are added to urlpatterns using include() should be
+ # reversable by doted path.
+ self.assertEqual(reverse('urlpatterns_reverse.views.nested_view'), '/includes/nested_path/')
def test_redirect_view_object(self):
from .views import absolute_kwargs_view
diff --git a/tests/urlpatterns_reverse/urls.py b/tests/urlpatterns_reverse/urls.py
index 923d0c7558..c79715e5bd 100644
--- a/tests/urlpatterns_reverse/urls.py
+++ b/tests/urlpatterns_reverse/urls.py
@@ -5,6 +5,7 @@ from .views import empty_view, absolute_kwargs_view
other_patterns = patterns('',
url(r'non_path_include/$', empty_view, name='non_path_include'),
+ url(r'nested_path/$', 'urlpatterns_reverse.views.nested_view'),
)
urlpatterns = patterns('',
diff --git a/tests/urlpatterns_reverse/views.py b/tests/urlpatterns_reverse/views.py
index f23b4bf257..610feb8451 100644
--- a/tests/urlpatterns_reverse/views.py
+++ b/tests/urlpatterns_reverse/views.py
@@ -21,6 +21,10 @@ def defaults_view(request, arg1, arg2):
pass
+def nested_view(request):
+ pass
+
+
def erroneous_view(request):
import non_existent # NOQA