Skip to content

Commit

Permalink
[1.6.x] Fixed a remote code execution vulnerabilty in URL reversing.
Browse files Browse the repository at this point in the history
Thanks Benjamin Bach for the report and initial patch.

This is a security fix; disclosure to follow shortly.

Backport of 8b93b31 from master
  • Loading branch information
timgraham committed Apr 21, 2014
1 parent 25adac9 commit 4352a50
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 2 deletions.
21 changes: 20 additions & 1 deletion django/core/urlresolvers.py
Expand Up @@ -243,6 +243,10 @@ def __init__(self, regex, urlconf_name, default_kwargs=None, app_name=None, name
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):
Expand All @@ -260,6 +264,15 @@ def _populate(self):
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:]
Expand All @@ -280,6 +293,7 @@ def _populate(self):
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))
Expand All @@ -288,6 +302,7 @@ def _populate(self):
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):
Expand Down Expand Up @@ -380,8 +395,12 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
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)
Expand Down
3 changes: 3 additions & 0 deletions tests/urlpatterns_reverse/nonimported_module.py
@@ -0,0 +1,3 @@
def view(request):
"""Stub view"""
pass
23 changes: 22 additions & 1 deletion tests/urlpatterns_reverse/tests.py
@@ -1,8 +1,11 @@
# -*- coding: utf-8 -*-
""" """
Unit tests for reverse URL lookups. Unit tests for reverse URL lookups.
""" """
from __future__ import absolute_import, unicode_literals from __future__ import absolute_import, unicode_literals


import sys

from django.conf import settings from django.conf import settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist
Expand Down Expand Up @@ -313,6 +316,25 @@ def test_redirect_to_url(self):
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
Expand Down Expand Up @@ -641,4 +663,3 @@ def test_view_loading(self):
# swallow it. # swallow it.
self.assertRaises(AttributeError, get_callable, self.assertRaises(AttributeError, get_callable,
'urlpatterns_reverse.views_broken.i_am_broken') 'urlpatterns_reverse.views_broken.i_am_broken')

1 change: 1 addition & 0 deletions tests/urlpatterns_reverse/urls.py
Expand Up @@ -7,6 +7,7 @@


other_patterns = patterns('', other_patterns = 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'),
) )


urlpatterns = patterns('', urlpatterns = patterns('',
Expand Down
4 changes: 4 additions & 0 deletions tests/urlpatterns_reverse/views.py
Expand Up @@ -16,6 +16,10 @@ def absolute_kwargs_view(request, arg1=1, arg2=2):
def defaults_view(request, arg1, arg2): def defaults_view(request, arg1, arg2):
pass pass


def nested_view(request):
pass


def erroneous_view(request): def erroneous_view(request):
import non_existent import non_existent


Expand Down

0 comments on commit 4352a50

Please sign in to comment.