Skip to content

Commit

Permalink
[1.4.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 ca3927d commit c1a8c42
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 2 deletions.
22 changes: 21 additions & 1 deletion django/core/urlresolvers.py
Expand Up @@ -230,6 +230,10 @@ def __init__(self, regex, urlconf_name, default_kwargs=None, app_name=None, name
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):
return smart_str(u'<%s %s (%s:%s) %s>' % (self.__class__.__name__, self.urlconf_name, self.app_name, self.namespace, self.regex.pattern))
Expand All @@ -240,6 +244,15 @@ def _populate(self):
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:]
Expand All @@ -260,6 +273,7 @@ def _populate(self):
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))
Expand All @@ -268,6 +282,7 @@ def _populate(self):
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):
Expand Down Expand Up @@ -356,8 +371,13 @@ def reverse(self, lookup_view, *args, **kwargs):
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), e:
raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e))
possibilities = self.reverse_dict.getlist(lookup_view)
Expand Down
@@ -0,0 +1,3 @@
def view(request):
"""Stub view"""
pass
23 changes: 22 additions & 1 deletion tests/regressiontests/urlpatterns_reverse/tests.py
@@ -1,8 +1,11 @@
# -*- coding: utf-8 -*-
"""
Unit tests for reverse URL lookups.
"""
from __future__ import absolute_import

import sys

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured, ViewDoesNotExist
from django.core.urlresolvers import (reverse, resolve, NoReverseMatch,
Expand Down Expand Up @@ -267,6 +270,25 @@ def test_redirect_to_url(self):
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
Expand Down Expand Up @@ -510,4 +532,3 @@ def test_erroneous_resolve(self):
self.assertRaises(ViewDoesNotExist, self.client.get, '/missing_inner/')
self.assertRaises(ViewDoesNotExist, self.client.get, '/missing_outer/')
self.assertRaises(ViewDoesNotExist, self.client.get, '/uncallable/')

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

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('',
Expand Down
4 changes: 4 additions & 0 deletions tests/regressiontests/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):
pass

def nested_view(request):
pass


def erroneous_view(request):
import non_existent

Expand Down

0 comments on commit c1a8c42

Please sign in to comment.