catch unknown charset when decoding content-disposition params

CHANGES/13042.bugfix.rst

@@ -0,0 +1,4 @@
+Fixed :exc:`LookupError` (and an unguarded :exc:`UnicodeDecodeError`) escaping
+``Content-Disposition`` parsing when a multipart part supplies an extended
+parameter with an unknown charset, e.g. ``filename*=unknown-8bit''...``
+-- by :user:`arshsmith1`.

CONTRIBUTORS.txt

@@ -55,6 +55,7 @@ Anton Zhdan-Pushkin
 Arcadiy Ivanov
 Arie Bovenberg
 Arseny Timoniq
+Arshiya Tabasum
 Artem Yushkovskiy
 Arthur Darcet
 Ashutosh Kumar Singh

aiohttp/multipart.py

@@ -1,5 +1,6 @@
 import base64
 import binascii
+import builtins
 import json
 import re
 import sys
@@ -150,7 +151,10 @@ def unescape(text: str, *, chars: str = "".join(map(re.escape, CHAR))) -> str:
 
             try:
                 value = unquote(value, encoding, "strict")
-            except UnicodeDecodeError:  # pragma: nocover
+            except (builtins.LookupError, UnicodeDecodeError):
+                # The charset is attacker-controlled here; an unknown name
+                # raises the builtin LookupError (the bare name is shadowed in
+                # this module by payload.LookupError).
                 warnings.warn(BadContentDispositionParam(item))
                 continue
 
@@ -208,7 +212,14 @@ def content_disposition_filename(
         if "'" in value:
             encoding, _, value = value.split("'", 2)
             encoding = encoding or "utf-8"
-            return unquote(value, encoding, "strict")
+            try:
+                return unquote(value, encoding, "strict")
+            except (builtins.LookupError, UnicodeDecodeError):
+                # Both the charset name and the octets are attacker-controlled
+                # here; an unknown encoding raises the builtin LookupError
+                # (shadowed in this module by payload.LookupError) and
+                # undecodable bytes raise UnicodeDecodeError.
+                return None
         return value
 
 

tests/test_multipart_helpers.py

@@ -525,6 +525,24 @@ def test_attwithfn2231nbadpct2(self) -> None:
         assert "attachment" == disptype
         assert {} == params
 
+    def test_attwithfn2231unknowncharset(self) -> None:
+        # An unknown charset name is attacker-controlled and makes
+        # urllib.parse.unquote raise the builtin LookupError.
+        with pytest.warns(aiohttp.BadContentDispositionParam):
+            disptype, params = parse_content_disposition(
+                "attachment; filename*=unknown-8bit''foo-%c3%a4.html"
+            )
+        assert "attachment" == disptype
+        assert {} == params
+
+    def test_attwithfn2231undecodable(self) -> None:
+        with pytest.warns(aiohttp.BadContentDispositionParam):
+            disptype, params = parse_content_disposition(
+                "attachment; filename*=UTF-8''%ff.html"
+            )
+        assert "attachment" == disptype
+        assert {} == params
+
     def test_attwithfn2231dpct(self) -> None:
         disptype, params = parse_content_disposition(
             "attachment; filename*=UTF-8''A-%2541.html"
@@ -697,6 +715,14 @@ def test_attfncontenc(self) -> None:
         params = {"filename*0*": "UTF-8''foo-%c3%a4", "filename*1": ".html"}
         assert "foo-ä.html" == content_disposition_filename(params)
 
+    def test_attfncontenc_unknown_charset(self) -> None:
+        params = {"filename*0*": "unknown-8bit''foo-%c3%a4", "filename*1": ".html"}
+        assert content_disposition_filename(params) is None
+
+    def test_attfncontenc_undecodable(self) -> None:
+        params = {"filename*0*": "UTF-8''%ff", "filename*1": ".html"}
+        assert content_disposition_filename(params) is None
+
     def test_attfncontlz(self) -> None:
         params = {"filename*0": "foo", "filename*01": "bar"}
         assert "foo" == content_disposition_filename(params)