mirrors/django
1
0
Fork 0
mirror of https://github.com/django/django.git synced 2024-12-22 17:16:24 +00:00
Code Issues 25 Releases Wiki Activity

Fixed CVE-2021-28658 -- Fixed potential directory-traversal via uploaded files.

Browse Source 
Thanks Claude Paroz for the initial patch.
Thanks Dennis Brinkrolf for the report.
This commit is contained in:
Mariusz Felisiak 2021-03-16 10:19:00 +01:00
parent 78fea27f69
commit d4d800ca1a
9 changed files with 159 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
django/http/multipartparser.py
View File

@ -212,9 +212,8 @@ class MultiPartParser:
# This is a file, use the handler...
file_name = disposition.get('filename')
if file_name:
file_name = os.path.basename(file_name)
file_name = force_str(file_name, encoding, errors='replace')
file_name = self.IE_sanitize(html.unescape(file_name))
file_name = self.sanitize_file_name(file_name)
if not file_name:
continue
@ -306,9 +305,13 @@ class MultiPartParser:
self._files.appendlist(force_str(old_field_name, self._encoding, errors='replace'), file_obj)
break
def IE_sanitize(self, filename):
"""Cleanup filename from Internet Explorer full paths."""
return filename and filename[filename.rfind("\\") + 1:].strip()
def sanitize_file_name(self, file_name):
file_name = html.unescape(file_name)
# Cleanup Windows-style path separators.
file_name = file_name[file_name.rfind('\\') + 1:].strip()
return os.path.basename(file_name)
IE_sanitize = sanitize_file_name
def _close_files(self):
# Free up all file handles.

15
docs/releases/2.2.20.txt Normal file
View File

@ -0,0 +1,15 @@
===========================
Django 2.2.20 release notes
===========================
*April 6, 2021*
Django 2.2.20 fixes a security issue with severity "low" in 2.2.19.
CVE-2021-28658: Potential directory-traversal via uploaded files
================================================================
``MultiPartParser`` allowed directory-traversal via uploaded files with
suitably crafted file names.
Built-in upload handlers were not affected by this vulnerability.

15
docs/releases/3.0.14.txt Normal file
View File

@ -0,0 +1,15 @@
===========================
Django 3.0.14 release notes
===========================
*April 6, 2021*
Django 3.0.14 fixes a security issue with severity "low" in 3.0.13.
CVE-2021-28658: Potential directory-traversal via uploaded files
================================================================
``MultiPartParser`` allowed directory-traversal via uploaded files with
suitably crafted file names.
Built-in upload handlers were not affected by this vulnerability.

12
docs/releases/3.1.8.txt
View File

@ -2,9 +2,17 @@
Django 3.1.8 release notes
==========================
*Expected April 5, 2021*
*April 6, 2021*
Django 3.1.8 fixes several bugs in 3.1.7.
Django 3.1.8 fixes a security issue with severity "low" and a bug in 3.1.7.
CVE-2021-28658: Potential directory-traversal via uploaded files
================================================================
``MultiPartParser`` allowed directory-traversal via uploaded files with
suitably crafted file names.
Built-in upload handlers were not affected by this vulnerability.
Bugfixes
========

2
docs/releases/index.txt
View File

@ -54,6 +54,7 @@ versions of the documentation contain the release notes for any later releases.
.. toctree::
:maxdepth: 1
3.0.14
3.0.13
3.0.12
3.0.11
@ -74,6 +75,7 @@ versions of the documentation contain the release notes for any later releases.
.. toctree::
:maxdepth: 1
2.2.20
2.2.19
2.2.18
2.2.17

84
tests/file_uploads/tests.py
View File

@ -23,6 +23,22 @@ UNICODE_FILENAME = 'test-0123456789_中文_Orléans.jpg'
MEDIA_ROOT = sys_tempfile.mkdtemp()
UPLOAD_TO = os.path.join(MEDIA_ROOT, 'test_upload')
CANDIDATE_TRAVERSAL_FILE_NAMES = [
'/tmp/hax0rd.txt', # Absolute path, *nix-style.
'C:\\Windows\\hax0rd.txt', # Absolute path, win-style.
'C:/Windows/hax0rd.txt', # Absolute path, broken-style.
'\\tmp\\hax0rd.txt', # Absolute path, broken in a different way.
'/tmp\\hax0rd.txt', # Absolute path, broken by mixing.
'subdir/hax0rd.txt', # Descendant path, *nix-style.
'subdir\\hax0rd.txt', # Descendant path, win-style.
'sub/dir\\hax0rd.txt', # Descendant path, mixed.
'../../hax0rd.txt', # Relative path, *nix-style.
'..\\..\\hax0rd.txt', # Relative path, win-style.
'../..\\hax0rd.txt', # Relative path, mixed.
'../hax0rd.txt', # HTML entities.
'../hax0rd.txt', # HTML entities.
]
@override_settings(MEDIA_ROOT=MEDIA_ROOT, ROOT_URLCONF='file_uploads.urls', MIDDLEWARE=[])
class FileUploadTests(TestCase):
@ -251,22 +267,8 @@ class FileUploadTests(TestCase):
# a malicious payload with an invalid file name (containing os.sep or
# os.pardir). This similar to what an attacker would need to do when
# trying such an attack.
scary_file_names = [
"/tmp/hax0rd.txt", # Absolute path, *nix-style.
"C:\\Windows\\hax0rd.txt", # Absolute path, win-style.
"C:/Windows/hax0rd.txt", # Absolute path, broken-style.
"\\tmp\\hax0rd.txt", # Absolute path, broken in a different way.
"/tmp\\hax0rd.txt", # Absolute path, broken by mixing.
"subdir/hax0rd.txt", # Descendant path, *nix-style.
"subdir\\hax0rd.txt", # Descendant path, win-style.
"sub/dir\\hax0rd.txt", # Descendant path, mixed.
"../../hax0rd.txt", # Relative path, *nix-style.
"..\\..\\hax0rd.txt", # Relative path, win-style.
"../..\\hax0rd.txt" # Relative path, mixed.
]
payload = client.FakePayload()
for i, name in enumerate(scary_file_names):
for i, name in enumerate(CANDIDATE_TRAVERSAL_FILE_NAMES):
payload.write('\r\n'.join([
'--' + client.BOUNDARY,
'Content-Disposition: form-data; name="file%s"; filename="%s"' % (i, name),
@ -286,7 +288,7 @@ class FileUploadTests(TestCase):
response = self.client.request(**r)
# The filenames should have been sanitized by the time it got to the view.
received = response.json()
for i, name in enumerate(scary_file_names):
for i, name in enumerate(CANDIDATE_TRAVERSAL_FILE_NAMES):
got = received["file%s" % i]
self.assertEqual(got, "hax0rd.txt")
@ -597,6 +599,47 @@ class FileUploadTests(TestCase):
# shouldn't differ.
self.assertEqual(os.path.basename(obj.testfile.path), 'MiXeD_cAsE.txt')
def test_filename_traversal_upload(self):
os.makedirs(UPLOAD_TO, exist_ok=True)
self.addCleanup(shutil.rmtree, MEDIA_ROOT)
tests = [
'../test.txt',
'../test.txt',
]
for file_name in tests:
with self.subTest(file_name=file_name):
payload = client.FakePayload()
payload.write(
'\r\n'.join([
'--' + client.BOUNDARY,
'Content-Disposition: form-data; name="my_file"; '
'filename="%s";' % file_name,
'Content-Type: text/plain',
'',
'file contents.\r\n',
'\r\n--' + client.BOUNDARY + '--\r\n',
]),
)
r = {
'CONTENT_LENGTH': len(payload),
'CONTENT_TYPE': client.MULTIPART_CONTENT,
'PATH_INFO': '/upload_traversal/',
'REQUEST_METHOD': 'POST',
'wsgi.input': payload,
}
response = self.client.request(**r)
result = response.json()
self.assertEqual(response.status_code, 200)
self.assertEqual(result['file_name'], 'test.txt')
self.assertIs(
os.path.exists(os.path.join(MEDIA_ROOT, 'test.txt')),
False,
)
self.assertIs(
os.path.exists(os.path.join(UPLOAD_TO, 'test.txt')),
True,
)
@override_settings(MEDIA_ROOT=MEDIA_ROOT)
class DirectoryCreationTests(SimpleTestCase):
@ -666,6 +709,15 @@ class MultiParserTests(SimpleTestCase):
}, StringIO('x'), [], 'utf-8')
self.assertEqual(multipart_parser._content_length, 0)
def test_sanitize_file_name(self):
parser = MultiPartParser({
'CONTENT_TYPE': 'multipart/form-data; boundary=_foo',
'CONTENT_LENGTH': '1'
}, StringIO('x'), [], 'utf-8')
for file_name in CANDIDATE_TRAVERSAL_FILE_NAMES:
with self.subTest(file_name=file_name):
self.assertEqual(parser.sanitize_file_name(file_name), 'hax0rd.txt')
def test_rfc2231_parsing(self):
test_data = (
(b"Content-Type: application/x-stuff; title*=us-ascii'en-us'This%20is%20%2A%2A%2Afun%2A%2A%2A",

31
tests/file_uploads/uploadhandler.py
View File

@ -1,6 +1,8 @@
"""
Upload handlers to test the upload API.
"""
import os
from tempfile import NamedTemporaryFile
from django.core.files.uploadhandler import (
FileUploadHandler, StopUpload, TemporaryFileUploadHandler,
@ -43,3 +45,32 @@ class ErroringUploadHandler(FileUploadHandler):
"""A handler that raises an exception."""
def receive_data_chunk(self, raw_data, start):
raise CustomUploadError("Oops!")
class TraversalUploadHandler(FileUploadHandler):
"""A handler with potential directory-traversal vulnerability."""
def __init__(self, request=None):
from .views import UPLOAD_TO
super().__init__(request)
self.upload_dir = UPLOAD_TO
def file_complete(self, file_size):
self.file.seek(0)
self.file.size = file_size
with open(os.path.join(self.upload_dir, self.file_name), 'wb') as fp:
fp.write(self.file.read())
return self.file
def new_file(
self, field_name, file_name, content_type, content_length, charset=None,
content_type_extra=None,
):
super().new_file(
file_name, file_name, content_length, content_length, charset,
content_type_extra,
)
self.file = NamedTemporaryFile(suffix='.upload', dir=self.upload_dir)
def receive_data_chunk(self, raw_data, start):
self.file.write(raw_data)

1
tests/file_uploads/urls.py
View File

@ -4,6 +4,7 @@ from . import views
urlpatterns = [
path('upload/', views.file_upload_view),
path('upload_traversal/', views.file_upload_traversal_view),
path('verify/', views.file_upload_view_verify),
path('unicode_name/', views.file_upload_unicode_name),
path('echo/', views.file_upload_echo),

9
tests/file_uploads/views.py
View File

@ -9,6 +9,7 @@ from .models import FileModel
from .tests import UNICODE_FILENAME, UPLOAD_TO
from .uploadhandler import (
ErroringUploadHandler, QuotaUploadHandler, StopUploadTemporaryFileHandler,
TraversalUploadHandler,
)
@ -162,3 +163,11 @@ def file_upload_fd_closing(request, access):
if access == 't':
request.FILES # Trigger file parsing.
return HttpResponse()
def file_upload_traversal_view(request):
request.upload_handlers.insert(0, TraversalUploadHandler())
request.FILES # Trigger file parsing.
return JsonResponse(
{'file_name': request.upload_handlers[0].file_name},
)