From 2f16ff5a6cbd71fc6c50e88e4087f3657222e90e Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Sat, 4 Oct 2014 19:04:21 +0200 Subject: [PATCH] Fixed #23601 -- Ensured view exists in URLconf before importing it in admindocs. --- django/contrib/admindocs/views.py | 7 ++++--- django/core/urlresolvers.py | 7 ++++++- docs/releases/1.8.txt | 8 ++++++++ tests/admin_docs/tests.py | 11 +++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/django/contrib/admindocs/views.py b/django/contrib/admindocs/views.py index f2b6535ae5..6cd5df1e5c 100644 --- a/django/contrib/admindocs/views.py +++ b/django/contrib/admindocs/views.py @@ -143,10 +143,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 6cd612e208..eff7e078e4 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -353,6 +353,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 = [] @@ -430,7 +435,7 @@ class RegexURLResolver(LocaleRegexProvider): original_lookup = lookup_view 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.8.txt b/docs/releases/1.8.txt index 01e48b75e4..434e91e9a5 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -76,6 +76,14 @@ Minor features ` to control whether or not the full count of objects should be displayed on a filtered admin page. +:mod:`django.contrib.admindocs` +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +* The view to browse view details now checks 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. + :mod:`django.contrib.auth` ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/admin_docs/tests.py b/tests/admin_docs/tests.py index 0826dfc973..c410eff020 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 @@ -84,6 +85,16 @@ class AdminDocViewTests(AdminDocsTestCase): # 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(