From da85d76fd6ca846f3b0ff414e042ddb5e62e2e69 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Wed, 29 Jul 2009 02:57:43 +0000 Subject: [PATCH] [0.96.X] 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 backport of r11351. A full announcement will be forthcoming. git-svn-id: http://code.djangoproject.com/svn/django/branches/0.96-bugfixes@11354 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/core/management.py | 4 +- django/core/servers/basehttp.py | 32 +++++++++-- tests/regressiontests/servers/__init__.py | 0 tests/regressiontests/servers/models.py | 0 tests/regressiontests/servers/tests.py | 67 +++++++++++++++++++++++ 5 files changed, 95 insertions(+), 8 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.py b/django/core/management.py index 091c38b637..68c85c74ff 100644 --- a/django/core/management.py +++ b/django/core/management.py @@ -1192,9 +1192,7 @@ def runserver(addr, port, use_reloader=True, admin_media_dir=''): print "Development server is running at http://%s:%s/" % (addr, port) print "Quit the server with %s." % quit_command try: - import django - path = admin_media_dir 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 a16b8b675a..27051d41b1 100644 --- a/django/core/servers/basehttp.py +++ b/django/core/servers/basehttp.py @@ -11,6 +11,8 @@ from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer from types import ListType, StringType import os, re, sys, time, urllib +from django.utils._os import safe_join + __version__ = "0.1" __all__ = ['WSGIServer','WSGIRequestHandler','demo_app'] @@ -599,11 +601,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 @@ -614,19 +630,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: status = '200 OK' headers = {} 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)