From 8bee4604a1be8ca3f051fcec57296308271836be Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Wed, 29 Jul 2009 02:40:14 +0000 Subject: [PATCH] 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 --- django/core/management/commands/runserver.py | 3 +- django/core/servers/basehttp.py | 31 +++++++-- tests/regressiontests/servers/__init__.py | 0 tests/regressiontests/servers/models.py | 0 tests/regressiontests/servers/tests.py | 67 ++++++++++++++++++++ 5 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 tests/regressiontests/servers/__init__.py create mode 100644 tests/regressiontests/servers/models.py create mode 100644 tests/regressiontests/servers/tests.py diff --git a/django/core/management/commands/runserver.py b/django/core/management/commands/runserver.py index 16d75fd88b..fc2c694ab6 100644 --- a/django/core/management/commands/runserver.py +++ b/django/core/management/commands/runserver.py @@ -56,8 +56,7 @@ class Command(BaseCommand): translation.activate(settings.LANGUAGE_CODE) try: - path = admin_media_path or django.__path__[0] + '/contrib/admin/media' - handler = AdminMediaHandler(WSGIHandler(), path) + handler = AdminMediaHandler(WSGIHandler(), admin_media_path) run(addr, int(port), handler) except WSGIServerException, e: # Use helpful error messages instead of ugly tracebacks. diff --git a/django/core/servers/basehttp.py b/django/core/servers/basehttp.py index 6fed0acc59..f7c0a77f7c 100644 --- a/django/core/servers/basehttp.py +++ b/django/core/servers/basehttp.py @@ -16,6 +16,7 @@ import sys import urllib from django.utils.http import http_date +from django.utils._os import safe_join __version__ = "0.1" __all__ = ['WSGIServer','WSGIRequestHandler'] @@ -621,11 +622,25 @@ class AdminMediaHandler(object): self.application = application if not media_dir: import django - self.media_dir = django.__path__[0] + '/contrib/admin/media' + self.media_dir = \ + os.path.join(django.__path__[0], 'contrib', 'admin', 'media') else: self.media_dir = media_dir self.media_url = settings.ADMIN_MEDIA_PREFIX + def file_path(self, url): + """ + Returns the path to the media file on disk for the given URL. + + The passed URL is assumed to begin with ADMIN_MEDIA_PREFIX. If the + resultant file path is outside the media directory, then a ValueError + is raised. + """ + # Remove ADMIN_MEDIA_PREFIX. + relative_url = url[len(self.media_url):] + relative_path = urllib.url2pathname(relative_url) + return safe_join(self.media_dir, relative_path) + def __call__(self, environ, start_response): import os.path @@ -636,19 +651,25 @@ class AdminMediaHandler(object): return self.application(environ, start_response) # Find the admin file and serve it up, if it exists and is readable. - relative_url = environ['PATH_INFO'][len(self.media_url):] - file_path = os.path.join(self.media_dir, relative_url) + try: + file_path = self.file_path(environ['PATH_INFO']) + except ValueError: # Resulting file path was not valid. + status = '404 NOT FOUND' + headers = {'Content-type': 'text/plain'} + output = ['Page not found: %s' % environ['PATH_INFO']] + start_response(status, headers.items()) + return output if not os.path.exists(file_path): status = '404 NOT FOUND' headers = {'Content-type': 'text/plain'} - output = ['Page not found: %s' % file_path] + output = ['Page not found: %s' % environ['PATH_INFO']] else: try: fp = open(file_path, 'rb') except IOError: status = '401 UNAUTHORIZED' headers = {'Content-type': 'text/plain'} - output = ['Permission denied: %s' % file_path] + output = ['Permission denied: %s' % environ['PATH_INFO']] else: # This is a very simple implementation of conditional GET with # the Last-Modified header. It makes media files a bit speedier diff --git a/tests/regressiontests/servers/__init__.py b/tests/regressiontests/servers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/servers/models.py b/tests/regressiontests/servers/models.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/servers/tests.py b/tests/regressiontests/servers/tests.py new file mode 100644 index 0000000000..7621439032 --- /dev/null +++ b/tests/regressiontests/servers/tests.py @@ -0,0 +1,67 @@ +""" +Tests for django.core.servers. +""" + +import os + +import django +from django.test import TestCase +from django.core.handlers.wsgi import WSGIHandler +from django.core.servers.basehttp import AdminMediaHandler + + +class AdminMediaHandlerTests(TestCase): + + def setUp(self): + self.admin_media_file_path = \ + os.path.join(django.__path__[0], 'contrib', 'admin', 'media') + self.handler = AdminMediaHandler(WSGIHandler()) + + def test_media_urls(self): + """ + Tests that URLs that look like absolute file paths after the + settings.ADMIN_MEDIA_PREFIX don't turn into absolute file paths. + """ + # Cases that should work on all platforms. + data = ( + ('/media/css/base.css', ('css', 'base.css')), + ) + # Cases that should raise an exception. + bad_data = () + + # Add platform-specific cases. + if os.sep == '/': + data += ( + # URL, tuple of relative path parts. + ('/media/\\css/base.css', ('\\css', 'base.css')), + ) + bad_data += ( + '/media//css/base.css', + '/media////css/base.css', + '/media/../css/base.css', + ) + elif os.sep == '\\': + bad_data += ( + '/media/C:\css/base.css', + '/media//\\css/base.css', + '/media/\\css/base.css', + '/media/\\\\css/base.css' + ) + for url, path_tuple in data: + try: + output = self.handler.file_path(url) + except ValueError: + self.fail("Got a ValueError exception, but wasn't expecting" + " one. URL was: %s" % url) + rel_path = os.path.join(*path_tuple) + desired = os.path.normcase( + os.path.join(self.admin_media_file_path, rel_path)) + self.assertEqual(output, desired, + "Got: %s, Expected: %s, URL was: %s" % (output, desired, url)) + for url in bad_data: + try: + output = self.handler.file_path(url) + except ValueError: + continue + self.fail('URL: %s should have caused a ValueError exception.' + % url)