Merge pull request #211 from tari/196-rfd

Guard against reflected file download

Author: Natim

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 (

tests/packaging.py

@@ -1,4 +1,5 @@
 """Tests around project's distribution and packaging."""
+import importlib.metadata
 import os
 import unittest
 

tests/response.py

@@ -19,3 +19,16 @@ def test_content_disposition_encoding(self):
         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"]
+        )

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