mirror of
https://github.com/openembedded/meta-openembedded.git
synced 2025-07-19 15:29:08 +02:00
660e62b645
In Django 2.2 before 2.2.20, 3.0 before 3.0.14, and 3.1 before 3.1.8,
MultiPartParser allowed directory traversal via uploaded files with
suitably crafted file names. Built-in upload handlers were not affected
by this vulnerability.
References:
https://nvd.nist.gov/vuln/detail/CVE-2021-28658
Upstream patches:
4036d62bda
Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Signed-off-by: Trevor Gamblin <trevor.gamblin@windriver.com>
290 lines
11 KiB
Diff
290 lines
11 KiB
Diff
From 4036d62bda0e9e9f6172943794b744a454ca49c2 Mon Sep 17 00:00:00 2001
|
|
From: Mariusz Felisiak <felisiak.mariusz@gmail.com>
|
|
Date: Tue, 16 Mar 2021 10:19:00 +0100
|
|
Subject: [PATCH] Fixed CVE-2021-28658 -- Fixed potential directory-traversal
|
|
via uploaded files.
|
|
|
|
Thanks Claude Paroz for the initial patch.
|
|
Thanks Dennis Brinkrolf for the report.
|
|
|
|
Backport of d4d800ca1addc4141e03c5440a849bb64d1582cd from main.
|
|
|
|
Upstream-Status: Backport
|
|
CVE: CVE-2021-28658
|
|
|
|
Reference to upstream patch:
|
|
[https://github.com/django/django/commit/4036d62bda0e9e9f6172943794b744a454ca49c2]
|
|
|
|
[SG: Adapted stable/2.2.x patch for 2.2.16]
|
|
Signed-off-by: Stefan Ghinea <stefan.ghinea@windriver.com>
|
|
---
|
|
django/http/multipartparser.py | 13 ++++--
|
|
docs/releases/2.2.16.txt | 12 +++++
|
|
tests/file_uploads/tests.py | 72 ++++++++++++++++++++++-------
|
|
tests/file_uploads/uploadhandler.py | 31 +++++++++++++
|
|
tests/file_uploads/urls.py | 1 +
|
|
tests/file_uploads/views.py | 12 ++++-
|
|
6 files changed, 120 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/django/http/multipartparser.py b/django/http/multipartparser.py
|
|
index f6f12ca..5a9cca8 100644
|
|
--- a/django/http/multipartparser.py
|
|
+++ b/django/http/multipartparser.py
|
|
@@ -7,6 +7,7 @@ file upload handlers for processing.
|
|
import base64
|
|
import binascii
|
|
import cgi
|
|
+import os
|
|
from urllib.parse import unquote
|
|
|
|
from django.conf import settings
|
|
@@ -205,7 +206,7 @@ class MultiPartParser:
|
|
file_name = disposition.get('filename')
|
|
if file_name:
|
|
file_name = force_text(file_name, encoding, errors='replace')
|
|
- file_name = self.IE_sanitize(unescape_entities(file_name))
|
|
+ file_name = self.sanitize_file_name(file_name)
|
|
if not file_name:
|
|
continue
|
|
|
|
@@ -293,9 +294,13 @@ class MultiPartParser:
|
|
self._files.appendlist(force_text(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 = unescape_entities(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.
|
|
diff --git a/docs/releases/2.2.16.txt b/docs/releases/2.2.16.txt
|
|
index 31231fb..4b7021b 100644
|
|
--- a/docs/releases/2.2.16.txt
|
|
+++ b/docs/releases/2.2.16.txt
|
|
@@ -2,6 +2,18 @@
|
|
Django 2.2.16 release notes
|
|
===========================
|
|
|
|
+*April 6, 2021*
|
|
+
|
|
+Backported from Django 2.2.20 a fix for a security issue.
|
|
+
|
|
+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.
|
|
+
|
|
*September 1, 2020*
|
|
|
|
Django 2.2.16 fixes two security issues and two data loss bugs in 2.2.15.
|
|
diff --git a/tests/file_uploads/tests.py b/tests/file_uploads/tests.py
|
|
index ea4976d..2a08d1b 100644
|
|
--- a/tests/file_uploads/tests.py
|
|
+++ b/tests/file_uploads/tests.py
|
|
@@ -22,6 +22,21 @@ 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.
|
|
+]
|
|
+
|
|
|
|
@override_settings(MEDIA_ROOT=MEDIA_ROOT, ROOT_URLCONF='file_uploads.urls', MIDDLEWARE=[])
|
|
class FileUploadTests(TestCase):
|
|
@@ -205,22 +220,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),
|
|
@@ -240,7 +241,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")
|
|
|
|
@@ -518,6 +519,36 @@ 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)
|
|
+ file_name = '../test.txt',
|
|
+ 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):
|
|
@@ -591,6 +622,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",
|
|
diff --git a/tests/file_uploads/uploadhandler.py b/tests/file_uploads/uploadhandler.py
|
|
index 7c6199f..65d70c6 100644
|
|
--- a/tests/file_uploads/uploadhandler.py
|
|
+++ b/tests/file_uploads/uploadhandler.py
|
|
@@ -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
|
|
|
|
@@ -35,3 +37,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)
|
|
diff --git a/tests/file_uploads/urls.py b/tests/file_uploads/urls.py
|
|
index 3e7985d..eaac1da 100644
|
|
--- a/tests/file_uploads/urls.py
|
|
+++ b/tests/file_uploads/urls.py
|
|
@@ -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),
|
|
diff --git a/tests/file_uploads/views.py b/tests/file_uploads/views.py
|
|
index d4947e4..137c6f3 100644
|
|
--- a/tests/file_uploads/views.py
|
|
+++ b/tests/file_uploads/views.py
|
|
@@ -6,7 +6,9 @@ from django.http import HttpResponse, HttpResponseServerError, JsonResponse
|
|
|
|
from .models import FileModel
|
|
from .tests import UNICODE_FILENAME, UPLOAD_TO
|
|
-from .uploadhandler import ErroringUploadHandler, QuotaUploadHandler
|
|
+from .uploadhandler import (
|
|
+ ErroringUploadHandler, QuotaUploadHandler, TraversalUploadHandler,
|
|
+)
|
|
|
|
|
|
def file_upload_view(request):
|
|
@@ -158,3 +160,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},
|
|
+ )
|
|
--
|
|
2.17.1
|
|
|