From e2b4470c5b624d7908fdb197df7e0c7749b8f007 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Tue, 30 Jul 2024 11:46:17 +0000 Subject: [PATCH 1/3] Ensure tests are actually run pytest by default only discovers tests in files named test_*.py, so none of the tests were actually being executed. Set the appropriate pytest option to discover the tests so they are automatically run. --- tox.ini | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index 612cd5d..d8e4eb3 100644 --- a/tox.ini +++ b/tox.ini @@ -31,10 +31,10 @@ deps = commands = pip install -e . pip install -e demo - # doctests + # doctests and unit tests pytest --cov=django_downloadview --cov=demoproject {posargs} - # all other test cases - coverage run --append {envbindir}/demo test {posargs: tests demoproject} + # demo project integration tests + coverage run --append {envbindir}/demo test {posargs: demoproject} coverage xml pip freeze ignore_outcome = @@ -76,3 +76,4 @@ source = django_downloadview,demo [pytest] DJANGO_SETTINGS_MODULE = demoproject.settings addopts = --doctest-modules --ignore=docs/ +python_files = tests/*.py From 0568c3c5593b302b9a7ef046775767d9eaa464b0 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Tue, 30 Jul 2024 11:51:25 +0000 Subject: [PATCH 2/3] Prevent reflected file downloads on specially-named files This fixes #196, where it was observed that django_downloadview was vulnerable to reflected file download attacks with specially-named files, similar to CVE-2022-36359 in Django. This change adopts the same replacement rules as used in Django's fix in commit b3e4494d759202a3b6bf247fd34455bf13be5b80. --- django_downloadview/response.py | 9 ++++++++- tests/response.py | 13 +++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/django_downloadview/response.py b/django_downloadview/response.py index 3c9ae9d..6648aaa 100644 --- a/django_downloadview/response.py +++ b/django_downloadview/response.py @@ -72,9 +72,16 @@ def content_disposition(filename): """ if not filename: return "attachment" - ascii_filename = encode_basename_ascii(filename) + # ASCII filenames are quoted and must ensure escape sequences + # in the filename won't break out of the quoted header value + # which can permit a reflected file download attack. The UTF-8 + # version is immune because it's not quoted. + ascii_filename = ( + encode_basename_ascii(filename).replace("\\", "\\\\").replace('"', r'\"') + ) utf8_filename = encode_basename_utf8(filename) if ascii_filename == utf8_filename: # ASCII only. + return f'attachment; filename="{ascii_filename}"' else: return ( diff --git a/tests/response.py b/tests/response.py index d87ce2b..738d364 100644 --- a/tests/response.py +++ b/tests/response.py @@ -19,3 +19,16 @@ class DownloadResponseTestCase(unittest.TestCase): self.assertIn( "filename*=UTF-8''espac%C3%A9%20.txt", headers["Content-Disposition"] ) + + def test_content_disposition_escaping(self): + """Content-Disposition headers escape special characters.""" + response = DownloadResponse( + "fake file", + attachment=True, + basename=r'"malicious\file.exe' + ) + headers = response.default_headers + self.assertIn( + r'filename="\"malicious\\file.exe"', + headers["Content-Disposition"] + ) \ No newline at end of file From e7e25e68ddbb275e8dfdfff4ea176eaea0079558 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Thu, 1 Aug 2024 06:28:06 +0000 Subject: [PATCH 3/3] Add missing import in packaging test This test was broken when changed to begin using importlib, but that wasn't evident because the tests directory wasn't being automatically tested. --- tests/packaging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/packaging.py b/tests/packaging.py index 42e9efe..ce2b705 100644 --- a/tests/packaging.py +++ b/tests/packaging.py @@ -1,4 +1,5 @@ """Tests around project's distribution and packaging.""" +import importlib.metadata import os import unittest