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.
This commit is contained in:
Tim Graham 2014-04-20 13:12:43 -04:00
parent ab90c4707b
commit 8b93b31487
5 changed files with 49 additions and 1 deletions

View File

@ -245,6 +245,10 @@ class RegexURLResolver(LocaleRegexProvider):
self._reverse_dict = {} self._reverse_dict = {}
self._namespace_dict = {} self._namespace_dict = {}
self._app_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): def __repr__(self):
if isinstance(self.urlconf_name, list) and len(self.urlconf_name): if isinstance(self.urlconf_name, list) and len(self.urlconf_name):
@ -262,6 +266,15 @@ class RegexURLResolver(LocaleRegexProvider):
apps = {} apps = {}
language_code = get_language() language_code = get_language()
for pattern in reversed(self.url_patterns): 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 p_pattern = pattern.regex.pattern
if p_pattern.startswith('^'): if p_pattern.startswith('^'):
p_pattern = p_pattern[1:] p_pattern = p_pattern[1:]
@ -280,6 +293,7 @@ class RegexURLResolver(LocaleRegexProvider):
namespaces[namespace] = (p_pattern + prefix, sub_pattern) namespaces[namespace] = (p_pattern + prefix, sub_pattern)
for app_name, namespace_list in pattern.app_dict.items(): for app_name, namespace_list in pattern.app_dict.items():
apps.setdefault(app_name, []).extend(namespace_list) apps.setdefault(app_name, []).extend(namespace_list)
self._callback_strs.update(pattern._callback_strs)
else: else:
bits = normalize(p_pattern) bits = normalize(p_pattern)
lookups.appendlist(pattern.callback, (bits, p_pattern, pattern.default_args)) lookups.appendlist(pattern.callback, (bits, p_pattern, pattern.default_args))
@ -288,6 +302,7 @@ class RegexURLResolver(LocaleRegexProvider):
self._reverse_dict[language_code] = lookups self._reverse_dict[language_code] = lookups
self._namespace_dict[language_code] = namespaces self._namespace_dict[language_code] = namespaces
self._app_dict[language_code] = apps self._app_dict[language_code] = apps
self._populated = True
@property @property
def reverse_dict(self): def reverse_dict(self):
@ -387,8 +402,12 @@ class RegexURLResolver(LocaleRegexProvider):
text_args = [force_text(v) for v in args] text_args = [force_text(v) for v in args]
text_kwargs = dict((k, force_text(v)) for (k, v) in kwargs.items()) text_kwargs = dict((k, force_text(v)) for (k, v) in kwargs.items())
if not self._populated:
self._populate()
try: 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: except (ImportError, AttributeError) as e:
raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e)) raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e))
possibilities = self.reverse_dict.getlist(lookup_view) possibilities = self.reverse_dict.getlist(lookup_view)

View File

@ -0,0 +1,3 @@
def view(request):
"""Stub view"""
pass

View File

@ -1,8 +1,10 @@
# -*- coding: utf-8 -*-
""" """
Unit tests for reverse URL lookups. Unit tests for reverse URL lookups.
""" """
from __future__ import unicode_literals from __future__ import unicode_literals
import sys
import unittest import unittest
from django.contrib.auth.models import User from django.contrib.auth.models import User
@ -356,6 +358,25 @@ class ReverseShortcutTests(TestCase):
self.assertEqual(res.url, '/foo/') self.assertEqual(res.url, '/foo/')
res = redirect('http://example.com/') res = redirect('http://example.com/')
self.assertEqual(res.url, '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): def test_redirect_view_object(self):
from .views import absolute_kwargs_view from .views import absolute_kwargs_view

View File

@ -7,6 +7,7 @@ from .views import empty_view, absolute_kwargs_view
other_patterns = [ other_patterns = [
url(r'non_path_include/$', empty_view, name='non_path_include'), url(r'non_path_include/$', empty_view, name='non_path_include'),
url(r'nested_path/$', 'urlpatterns_reverse.views.nested_view'),
] ]
# test deprecated patterns() function. convert to list of urls() in Django 2.0 # test deprecated patterns() function. convert to list of urls() in Django 2.0

View File

@ -21,6 +21,10 @@ def defaults_view(request, arg1, arg2):
pass pass
def nested_view(request):
pass
def erroneous_view(request): def erroneous_view(request):
import non_existent # NOQA import non_existent # NOQA