Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

SECURITY ALERT: Corrected a problem with the Admin media handler that…

… could lead to the exposure of system files. Thanks to Gary Wilson for the patch.

This is a security-related update. A full announcement, as well as backports for 1.0.X and 0.96.X will be forthcoming.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@11351 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 8bee4604a1be8ca3f051fcec57296308271836be 1 parent d1cb1eb
Russell Keith-Magee authored July 29, 2009
3  django/core/management/commands/runserver.py
@@ -56,8 +56,7 @@ def inner_run():
56 56
             translation.activate(settings.LANGUAGE_CODE)
57 57
 
58 58
             try:
59  
-                path = admin_media_path or django.__path__[0] + '/contrib/admin/media'
60  
-                handler = AdminMediaHandler(WSGIHandler(), path)
  59
+                handler = AdminMediaHandler(WSGIHandler(), admin_media_path)
61 60
                 run(addr, int(port), handler)
62 61
             except WSGIServerException, e:
63 62
                 # Use helpful error messages instead of ugly tracebacks.
31  django/core/servers/basehttp.py
@@ -16,6 +16,7 @@
16 16
 import urllib
17 17
 
18 18
 from django.utils.http import http_date
  19
+from django.utils._os import safe_join
19 20
 
20 21
 __version__ = "0.1"
21 22
 __all__ = ['WSGIServer','WSGIRequestHandler']
@@ -621,11 +622,25 @@ def __init__(self, application, media_dir=None):
621 622
         self.application = application
622 623
         if not media_dir:
623 624
             import django
624  
-            self.media_dir = django.__path__[0] + '/contrib/admin/media'
  625
+            self.media_dir = \
  626
+                os.path.join(django.__path__[0], 'contrib', 'admin', 'media')
625 627
         else:
626 628
             self.media_dir = media_dir
627 629
         self.media_url = settings.ADMIN_MEDIA_PREFIX
628 630
 
  631
+    def file_path(self, url):
  632
+        """
  633
+        Returns the path to the media file on disk for the given URL.
  634
+
  635
+        The passed URL is assumed to begin with ADMIN_MEDIA_PREFIX.  If the
  636
+        resultant file path is outside the media directory, then a ValueError
  637
+        is raised.
  638
+        """
  639
+        # Remove ADMIN_MEDIA_PREFIX.
  640
+        relative_url = url[len(self.media_url):]
  641
+        relative_path = urllib.url2pathname(relative_url)
  642
+        return safe_join(self.media_dir, relative_path)
  643
+
629 644
     def __call__(self, environ, start_response):
630 645
         import os.path
631 646
 
@@ -636,19 +651,25 @@ def __call__(self, environ, start_response):
636 651
             return self.application(environ, start_response)
637 652
 
638 653
         # Find the admin file and serve it up, if it exists and is readable.
639  
-        relative_url = environ['PATH_INFO'][len(self.media_url):]
640  
-        file_path = os.path.join(self.media_dir, relative_url)
  654
+        try:
  655
+            file_path = self.file_path(environ['PATH_INFO'])
  656
+        except ValueError: # Resulting file path was not valid.
  657
+            status = '404 NOT FOUND'
  658
+            headers = {'Content-type': 'text/plain'}
  659
+            output = ['Page not found: %s' % environ['PATH_INFO']]
  660
+            start_response(status, headers.items())
  661
+            return output
641 662
         if not os.path.exists(file_path):
642 663
             status = '404 NOT FOUND'
643 664
             headers = {'Content-type': 'text/plain'}
644  
-            output = ['Page not found: %s' % file_path]
  665
+            output = ['Page not found: %s' % environ['PATH_INFO']]
645 666
         else:
646 667
             try:
647 668
                 fp = open(file_path, 'rb')
648 669
             except IOError:
649 670
                 status = '401 UNAUTHORIZED'
650 671
                 headers = {'Content-type': 'text/plain'}
651  
-                output = ['Permission denied: %s' % file_path]
  672
+                output = ['Permission denied: %s' % environ['PATH_INFO']]
652 673
             else:
653 674
                 # This is a very simple implementation of conditional GET with
654 675
                 # the Last-Modified header. It makes media files a bit speedier
0  tests/regressiontests/servers/__init__.py
No changes.
0  tests/regressiontests/servers/models.py
No changes.
67  tests/regressiontests/servers/tests.py
... ...
@@ -0,0 +1,67 @@
  1
+"""
  2
+Tests for django.core.servers.
  3
+"""
  4
+
  5
+import os
  6
+
  7
+import django
  8
+from django.test import TestCase
  9
+from django.core.handlers.wsgi import WSGIHandler
  10
+from django.core.servers.basehttp import AdminMediaHandler
  11
+
  12
+
  13
+class AdminMediaHandlerTests(TestCase):
  14
+
  15
+    def setUp(self):
  16
+        self.admin_media_file_path = \
  17
+            os.path.join(django.__path__[0], 'contrib', 'admin', 'media')
  18
+        self.handler = AdminMediaHandler(WSGIHandler())
  19
+
  20
+    def test_media_urls(self):
  21
+        """
  22
+        Tests that URLs that look like absolute file paths after the
  23
+        settings.ADMIN_MEDIA_PREFIX don't turn into absolute file paths.
  24
+        """
  25
+        # Cases that should work on all platforms.
  26
+        data = (
  27
+            ('/media/css/base.css', ('css', 'base.css')),
  28
+        )
  29
+        # Cases that should raise an exception.
  30
+        bad_data = ()
  31
+
  32
+        # Add platform-specific cases.
  33
+        if os.sep == '/':
  34
+            data += (
  35
+                # URL, tuple of relative path parts.
  36
+                ('/media/\\css/base.css', ('\\css', 'base.css')),
  37
+            )
  38
+            bad_data += (
  39
+                '/media//css/base.css',
  40
+                '/media////css/base.css',
  41
+                '/media/../css/base.css',
  42
+            )
  43
+        elif os.sep == '\\':
  44
+            bad_data += (
  45
+                '/media/C:\css/base.css',
  46
+                '/media//\\css/base.css',
  47
+                '/media/\\css/base.css',
  48
+                '/media/\\\\css/base.css'
  49
+            )
  50
+        for url, path_tuple in data:
  51
+            try:
  52
+                output = self.handler.file_path(url)
  53
+            except ValueError:
  54
+                self.fail("Got a ValueError exception, but wasn't expecting"
  55
+                          " one. URL was: %s" % url)
  56
+            rel_path = os.path.join(*path_tuple)
  57
+            desired = os.path.normcase(
  58
+                os.path.join(self.admin_media_file_path, rel_path))
  59
+            self.assertEqual(output, desired,
  60
+                "Got: %s, Expected: %s, URL was: %s" % (output, desired, url))
  61
+        for url in bad_data:
  62
+            try:
  63
+                output = self.handler.file_path(url)
  64
+            except ValueError:
  65
+                continue
  66
+            self.fail('URL: %s should have caused a ValueError exception.'
  67
+                      % url)

0 notes on commit 8bee460

Please sign in to comment.
Something went wrong with that request. Please try again.