diff options
| author | Tim Graham <timograham@gmail.com> | 2014-04-20 13:33:59 -0400 |
|---|---|---|
| committer | Tim Graham <timograham@gmail.com> | 2014-04-21 18:30:57 -0400 |
| commit | 2a5bcb69f42b84464b24b5c835dca6467b6aa7f1 (patch) | |
| tree | dec2949f78628fcfcf2c3cd946720302ef04bbe3 | |
| parent | d6c685cc78d6bde96a60d82b9a065db3cd44ac66 (diff) | |
[1.5.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.py | 22 | ||||
| -rw-r--r-- | tests/regressiontests/urlpatterns_reverse/nonimported_module.py | 3 | ||||
| -rw-r--r-- | tests/regressiontests/urlpatterns_reverse/tests.py | 23 | ||||
| -rw-r--r-- | tests/regressiontests/urlpatterns_reverse/urls.py | 1 | ||||
| -rw-r--r-- | tests/regressiontests/urlpatterns_reverse/views.py | 4 |
5 files changed, 51 insertions, 2 deletions
diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index c657fd9a54..17cad51db5 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -244,6 +244,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): @@ -261,6 +265,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:] @@ -281,6 +294,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)) @@ -289,6 +303,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): @@ -375,8 +390,13 @@ class RegexURLResolver(LocaleRegexProvider): def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs): if args and kwargs: raise ValueError("Don't mix *args and **kwargs in call to reverse()!") + + 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/regressiontests/urlpatterns_reverse/nonimported_module.py b/tests/regressiontests/urlpatterns_reverse/nonimported_module.py new file mode 100644 index 0000000000..df046333d3 --- /dev/null +++ b/tests/regressiontests/urlpatterns_reverse/nonimported_module.py @@ -0,0 +1,3 @@ +def view(request): + """Stub view""" + pass diff --git a/tests/regressiontests/urlpatterns_reverse/tests.py b/tests/regressiontests/urlpatterns_reverse/tests.py index eb3afe8201..e3e14b3d7c 100644 --- a/tests/regressiontests/urlpatterns_reverse/tests.py +++ b/tests/regressiontests/urlpatterns_reverse/tests.py @@ -1,8 +1,11 @@ +# -*- coding: utf-8 -*- """ Unit tests for reverse URL lookups. """ from __future__ import absolute_import, unicode_literals +import sys + from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist @@ -290,6 +293,25 @@ class ReverseShortcutTests(TestCase): self.assertEqual(res['Location'], '/foo/') res = redirect('http://example.com/') self.assertEqual(res['Location'], 'http://example.com/') + # Assert that we can redirect using UTF-8 strings + res = redirect('/æøå/abc/') + self.assertEqual(res['Location'], '/%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['Location'], '/%C3%A6%C3%B8%C3%A5.abc/') + res = redirect('os.path') + self.assertEqual(res['Location'], '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('regressiontests.urlpatterns_reverse.views.nested_view'), '/includes/nested_path/') def test_redirect_view_object(self): from .views import absolute_kwargs_view @@ -559,4 +581,3 @@ class ViewLoadingTests(TestCase): # swallow it. self.assertRaises(AttributeError, get_callable, 'regressiontests.urlpatterns_reverse.views_broken.i_am_broken') - diff --git a/tests/regressiontests/urlpatterns_reverse/urls.py b/tests/regressiontests/urlpatterns_reverse/urls.py index 1d4ae73c67..2af306b597 100644 --- a/tests/regressiontests/urlpatterns_reverse/urls.py +++ b/tests/regressiontests/urlpatterns_reverse/urls.py @@ -7,6 +7,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/$', 'regressiontests.urlpatterns_reverse.views.nested_view'), ) urlpatterns = patterns('', diff --git a/tests/regressiontests/urlpatterns_reverse/views.py b/tests/regressiontests/urlpatterns_reverse/views.py index 88d169a118..f8148e1f3e 100644 --- a/tests/regressiontests/urlpatterns_reverse/views.py +++ b/tests/regressiontests/urlpatterns_reverse/views.py @@ -16,6 +16,10 @@ def absolute_kwargs_view(request, arg1=1, arg2=2): def defaults_view(request, arg1, arg2): pass +def nested_view(request): + pass + + def erroneous_view(request): import non_existent |
