[1.7.x] Fixed #23601 -- Ensured view exists in URLconf before importing it in admindocs.

MarkusH · timgraham · commit c2508990cb53 · 2014-10-06T13:44:01.000-04:00
Backport of 2f16ff5 from master
diff --git 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
@@ -329,6 +329,11 @@ def app_dict(self):
             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 @@ def _reverse_with_prefix(self, lookup_view, _prefix, *args, **kwargs):
             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
@@ -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
@@ -1,3 +1,4 @@
+import sys
 import unittest
 
 from django.conf import settings
@@ -79,6 +80,16 @@ def test_view_detail(self):
         # 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(
