Skip to content

Commit da85d76

Browse files
freakboy3742freakboy3742
committed
[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
1 parent 1232a46 commit da85d76

File tree

5 files changed

+95
-8
lines changed

5 files changed

+95
-8
lines changed

‎django/core/management.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,9 +1192,7 @@ def inner_run():
11921192
print "Development server is running at http://%s:%s/" % (addr, port)
11931193
print "Quit the server with %s." % quit_command
11941194
try:
1195-
import django
1196-
path = admin_media_dir or django.__path__[0] + '/contrib/admin/media'
1197-
handler = AdminMediaHandler(WSGIHandler(), path)
1195+
handler = AdminMediaHandler(WSGIHandler(), admin_media_path)
11981196
run(addr, int(port), handler)
11991197
except WSGIServerException, e:
12001198
# Use helpful error messages instead of ugly tracebacks.

‎django/core/servers/basehttp.py‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
from types import ListType, StringType
1212
import os, re, sys, time, urllib
1313

14+
from django.utils._os import safe_join
15+
1416
__version__ = "0.1"
1517
__all__ = ['WSGIServer','WSGIRequestHandler','demo_app']
1618

@@ -599,11 +601,25 @@ def __init__(self, application, media_dir=None):
599601
self.application = application
600602
if not media_dir:
601603
import django
602-
self.media_dir = django.__path__[0] + '/contrib/admin/media'
604+
self.media_dir = \
605+
os.path.join(django.__path__[0], 'contrib', 'admin', 'media')
603606
else:
604607
self.media_dir = media_dir
605608
self.media_url = settings.ADMIN_MEDIA_PREFIX
606609

610+
def file_path(self, url):
611+
"""
612+
Returns the path to the media file on disk for the given URL.
613+
614+
The passed URL is assumed to begin with ADMIN_MEDIA_PREFIX. If the
615+
resultant file path is outside the media directory, then a ValueError
616+
is raised.
617+
"""
618+
# Remove ADMIN_MEDIA_PREFIX.
619+
relative_url = url[len(self.media_url):]
620+
relative_path = urllib.url2pathname(relative_url)
621+
return safe_join(self.media_dir, relative_path)
622+
607623
def __call__(self, environ, start_response):
608624
import os.path
609625

@@ -614,19 +630,25 @@ def __call__(self, environ, start_response):
614630
return self.application(environ, start_response)
615631

616632
# Find the admin file and serve it up, if it exists and is readable.
617-
relative_url = environ['PATH_INFO'][len(self.media_url):]
618-
file_path = os.path.join(self.media_dir, relative_url)
633+
try:
634+
file_path = self.file_path(environ['PATH_INFO'])
635+
except ValueError: # Resulting file path was not valid.
636+
status = '404 NOT FOUND'
637+
headers = {'Content-type': 'text/plain'}
638+
output = ['Page not found: %s' % environ['PATH_INFO']]
639+
start_response(status, headers.items())
640+
return output
619641
if not os.path.exists(file_path):
620642
status = '404 NOT FOUND'
621643
headers = {'Content-type': 'text/plain'}
622-
output = ['Page not found: %s' % file_path]
644+
output = ['Page not found: %s' % environ['PATH_INFO']]
623645
else:
624646
try:
625647
fp = open(file_path, 'rb')
626648
except IOError:
627649
status = '401 UNAUTHORIZED'
628650
headers = {'Content-type': 'text/plain'}
629-
output = ['Permission denied: %s' % file_path]
651+
output = ['Permission denied: %s' % environ['PATH_INFO']]
630652
else:
631653
status = '200 OK'
632654
headers = {}

‎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)