Skip to content

Commit df7f917

Browse files
freakboy3742freakboy3742
committed
[1.0.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, as well as a backport 0.96.X will be forthcoming. git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.0.X@11353 bcc190cf-cafb-0310-a4f2-bffc1f526a37
1 parent f9249e4 commit df7f917

File tree

5 files changed

+94
-7
lines changed

5 files changed

+94
-7
lines changed

‎django/core/management/commands/runserver.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ def inner_run():
5656
translation.activate(settings.LANGUAGE_CODE)
5757

5858
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)
6160
run(addr, int(port), handler)
6261
except WSGIServerException, e:
6362
# Use helpful error messages instead of ugly tracebacks.

‎django/core/servers/basehttp.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import urllib
1717

1818
from django.utils.http import http_date
19+
from django.utils._os import safe_join
1920

2021
__version__ = "0.1"
2122
__all__ = ['WSGIServer','WSGIRequestHandler']
@@ -621,11 +622,25 @@ def __init__(self, application, media_dir=None):
621622
self.application = application
622623
if not media_dir:
623624
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')
625627
else:
626628
self.media_dir = media_dir
627629
self.media_url = settings.ADMIN_MEDIA_PREFIX
628630

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+
629644
def __call__(self, environ, start_response):
630645
import os.path
631646

@@ -636,19 +651,25 @@ def __call__(self, environ, start_response):
636651
return self.application(environ, start_response)
637652

638653
# 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
641662
if not os.path.exists(file_path):
642663
status = '404 NOT FOUND'
643664
headers = {'Content-type': 'text/plain'}
644-
output = ['Page not found: %s' % file_path]
665+
output = ['Page not found: %s' % environ['PATH_INFO']]
645666
else:
646667
try:
647668
fp = open(file_path, 'rb')
648669
except IOError:
649670
status = '401 UNAUTHORIZED'
650671
headers = {'Content-type': 'text/plain'}
651-
output = ['Permission denied: %s' % file_path]
672+
output = ['Permission denied: %s' % environ['PATH_INFO']]
652673
else:
653674
# This is a very simple implementation of conditional GET with
654675
# the Last-Modified header. It makes media files a bit speedier

‎tests/regressiontests/servers/__init__.py

Whitespace-only changes.

‎tests/regressiontests/servers/models.py

Whitespace-only changes.

‎tests/regressiontests/servers/tests.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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 commit comments

Comments
 (0)