From c2508990cb53b52783ebb38dc0b5f0ab5d023c76 Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Sat, 4 Oct 2014 19:04:21 +0200 Subject: [PATCH] [1.7.x] Fixed #23601 -- Ensured view exists in URLconf before importing it in admindocs. Backport of 2f16ff5a6c from master --- django/contrib/admindocs/views.py | 7 ++++--- django/core/urlresolvers.py | 7 ++++++- docs/releases/1.7.1.txt | 6 ++++++ tests/admin_docs/tests.py | 11 +++++++++++ 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/django/contrib/admindocs/views.py b/django/contrib/admindocs/views.py index 6cb46beaf5..06f8844c03 100644 --- a/django/contrib/admindocs/views.py +++ b/django/contrib/admindocs/views.py @@ -150,10 +150,11 @@ class ViewDetailView(BaseAdminDocsView): def get_context_data(self, **kwargs): view = self.kwargs['view'] - mod, func = urlresolvers.get_mod_func(view) - try: + urlconf = urlresolvers.get_urlconf() + if urlresolvers.get_resolver(urlconf)._is_callback(view): + mod, func = urlresolvers.get_mod_func(view) view_func = getattr(import_module(mod), func) - except (ImportError, AttributeError): + else: raise Http404 title, body, metadata = utils.parse_docstring(view_func.__doc__) if title: diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index 3417ae3eab..ec2202edb7 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -329,6 +329,11 @@ class RegexURLResolver(LocaleRegexProvider): self._populate() return self._app_dict[language_code] + def _is_callback(self, name): + if not self._populated: + self._populate() + return name in self._callback_strs + def resolve(self, path): path = force_text(path) # path may be a reverse_lazy object tried = [] @@ -410,7 +415,7 @@ class RegexURLResolver(LocaleRegexProvider): self._populate() try: - if lookup_view in self._callback_strs: + if self._is_callback(lookup_view): lookup_view = get_callable(lookup_view, True) except (ImportError, AttributeError) as e: raise NoReverseMatch("Error importing '%s': %s." % (lookup_view, e)) diff --git a/docs/releases/1.7.1.txt b/docs/releases/1.7.1.txt index f743447d56..0a5a0163b8 100644 --- a/docs/releases/1.7.1.txt +++ b/docs/releases/1.7.1.txt @@ -91,3 +91,9 @@ Bugfixes (:ticket:`23560`). * Fixed ``deepcopy`` on ``ErrorList`` (:ticket:`23594`). + +* Made the :mod:`~django.contrib.admindocs` view to browse view details check + if the view specified in the URL exists in the URLconf. Previously it was + possible to import arbitrary packages from the Python path. This was not + considered a security issue because ``admindocs`` is only accessible to staff + users (:ticket:`23601`). diff --git a/tests/admin_docs/tests.py b/tests/admin_docs/tests.py index 93e49e0eb5..5808d91a95 100644 --- a/tests/admin_docs/tests.py +++ b/tests/admin_docs/tests.py @@ -1,3 +1,4 @@ +import sys import unittest from django.conf import settings @@ -79,6 +80,16 @@ class AdminDocViewTests(TestCase): # View docstring self.assertContains(response, 'Base view for admindocs views.') + def test_view_detail_illegal_import(self): + """ + #23601 - Ensure the view exists in the URLconf. + """ + response = self.client.get( + reverse('django-admindocs-views-detail', + args=['urlpatterns_reverse.nonimported_module.view'])) + self.assertEqual(response.status_code, 404) + self.assertNotIn("urlpatterns_reverse.nonimported_module", sys.modules) + def test_model_index(self): response = self.client.get(reverse('django-admindocs-models-index')) self.assertContains(