Fix unsafe hotserving behaviour for non-multimedia uploads.

changelog.d/15680.bugfix

@@ -0,0 +1 @@
+Fix unsafe hotserving behaviour for non-multimedia uploads.

synapse/media/_base.py

@@ -50,6 +50,8 @@
     "text/xml",
 ]
 
+HOTSERVE_CONTENT_TYPES = ["audio/", "video/", "image/"]
+
 
 def parse_media_id(request: Request) -> Tuple[str, str, Optional[str]]:
     """Parses the server name, media ID and optional file name from the request URI
@@ -151,7 +153,16 @@ def _quote(x: str) -> str:
     else:
         content_type = media_type
 
+    # Only hotserve "safe" mimetypes, force download everything else
+    disposition_type = "attachment"
+    for mime in HOTSERVE_CONTENT_TYPES:
+        if media_type.lower().startswith(mime):
+            disposition_type = "inline"
+            break
+
+    disposition = disposition_type
     request.setHeader(b"Content-Type", content_type.encode("UTF-8"))
+
     if upload_name:
         # RFC6266 section 4.1 [1] defines both `filename` and `filename*`.
         #
@@ -173,11 +184,17 @@ def _quote(x: str) -> str:
         # correctly interpret those as of 0.99.2 and (b) they are a bit of a pain and we
         # may as well just do the filename* version.
         if _can_encode_filename_as_token(upload_name):
-            disposition = "inline; filename=%s" % (upload_name,)
+            disposition = "%s; filename=%s" % (
+                disposition_type,
+                upload_name,
+            )
         else:
-            disposition = "inline; filename*=utf-8''%s" % (_quote(upload_name),)
+            disposition = "%s; filename*=utf-8''%s" % (
+                disposition_type,
+                _quote(upload_name),
+            )
 
-        request.setHeader(b"Content-Disposition", disposition.encode("ascii"))
+    request.setHeader(b"Content-Disposition", disposition.encode("ascii"))
 
     # cache for at least a day.
     # XXX: we might want to turn this off for data we don't want to

tests/media/test_base.py

@@ -20,12 +20,12 @@
 class GetFileNameFromHeadersTests(unittest.TestCase):
     # input -> expected result
     TEST_CASES = {
-        b"inline; filename=abc.txt": "abc.txt",
-        b'inline; filename="azerty"': "azerty",
-        b'inline; filename="aze%20rty"': "aze%20rty",
-        b'inline; filename="aze"rty"': 'aze"rty',
-        b'inline; filename="azer;ty"': "azer;ty",
-        b"inline; filename*=utf-8''foo%C2%A3bar": "foo£bar",
+        b"attachment; filename=abc.txt": "abc.txt",
+        b'attachment; filename="azerty"': "azerty",
+        b'attachment; filename="aze%20rty"': "aze%20rty",
+        b'attachment; filename="aze"rty"': 'aze"rty',
+        b'attachment; filename="azer;ty"': "azer;ty",
+        b"attachment; filename*=utf-8''foo%C2%A3bar": "foo£bar",
     }
 
     def tests(self) -> None:

tests/media/test_media_storage.py

@@ -364,16 +364,16 @@ def test_disposition_filenamestar_utf8escaped(self) -> None:
 
     def test_disposition_none(self) -> None:
         """
-        If there is no filename, one isn't passed on in the Content-Disposition
-        of the request.
+        If there is no filename, Content-Disposition should only
+        be a disposition type.
         """
         channel = self._req(None)
 
         headers = channel.headers
         self.assertEqual(
             headers.getRawHeaders(b"Content-Type"), [self.test_image.content_type]
         )
-        self.assertEqual(headers.getRawHeaders(b"Content-Disposition"), None)
+        self.assertEqual(headers.getRawHeaders(b"Content-Disposition"), [b"inline"])
 
     def test_thumbnail_crop(self) -> None:
         """Test that a cropped remote thumbnail is available."""