mirror of
https://mau.dev/maunium/synapse.git
synced 2024-10-01 01:36:05 -04:00
Fix unsafe hotserving behaviour for non-multimedia uploads. (#15680)
* Fix unsafe hotserving behaviour for non-multimedia uploads. * invert disposition assert * test_media_storage.py: run lint * test_base.py: /inline/attachment/s * Only return attachment for disposition type, update tests * Update synapse/media/_base.py Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com> * Update changelog.d/15680.bugfix Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com> * add attribution * Update changelog. --------- Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
This commit is contained in:
parent
1404f68a03
commit
d939120421
1
changelog.d/15680.bugfix
Normal file
1
changelog.d/15680.bugfix
Normal file
@ -0,0 +1 @@
|
||||
Fix a long-standing bug where media files were served in an unsafe manner. Contributed by @joshqou.
|
@ -152,6 +152,9 @@ def add_file_headers(
|
||||
content_type = media_type
|
||||
|
||||
request.setHeader(b"Content-Type", content_type.encode("UTF-8"))
|
||||
|
||||
# Use a Content-Disposition of attachment to force download of media.
|
||||
disposition = "attachment"
|
||||
if upload_name:
|
||||
# RFC6266 section 4.1 [1] defines both `filename` and `filename*`.
|
||||
#
|
||||
@ -173,9 +176,15 @@ def add_file_headers(
|
||||
# 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,
|
||||
upload_name,
|
||||
)
|
||||
else:
|
||||
disposition = "inline; filename*=utf-8''%s" % (_quote(upload_name),)
|
||||
disposition = "%s; filename*=utf-8''%s" % (
|
||||
disposition,
|
||||
_quote(upload_name),
|
||||
)
|
||||
|
||||
request.setHeader(b"Content-Disposition", disposition.encode("ascii"))
|
||||
|
||||
|
@ -20,12 +20,12 @@ from tests import unittest
|
||||
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:
|
||||
|
@ -317,7 +317,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
|
||||
def test_handle_missing_content_type(self) -> None:
|
||||
channel = self._req(
|
||||
b"inline; filename=out" + self.test_image.extension,
|
||||
b"attachment; filename=out" + self.test_image.extension,
|
||||
include_content_type=False,
|
||||
)
|
||||
headers = channel.headers
|
||||
@ -331,7 +331,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
If the filename is filename=<ascii> then Synapse will decode it as an
|
||||
ASCII string, and use filename= in the response.
|
||||
"""
|
||||
channel = self._req(b"inline; filename=out" + self.test_image.extension)
|
||||
channel = self._req(b"attachment; filename=out" + self.test_image.extension)
|
||||
|
||||
headers = channel.headers
|
||||
self.assertEqual(
|
||||
@ -339,7 +339,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
)
|
||||
self.assertEqual(
|
||||
headers.getRawHeaders(b"Content-Disposition"),
|
||||
[b"inline; filename=out" + self.test_image.extension],
|
||||
[b"attachment; filename=out" + self.test_image.extension],
|
||||
)
|
||||
|
||||
def test_disposition_filenamestar_utf8escaped(self) -> None:
|
||||
@ -350,7 +350,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
"""
|
||||
filename = parse.quote("\u2603".encode()).encode("ascii")
|
||||
channel = self._req(
|
||||
b"inline; filename*=utf-8''" + filename + self.test_image.extension
|
||||
b"attachment; filename*=utf-8''" + filename + self.test_image.extension
|
||||
)
|
||||
|
||||
headers = channel.headers
|
||||
@ -359,13 +359,13 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
)
|
||||
self.assertEqual(
|
||||
headers.getRawHeaders(b"Content-Disposition"),
|
||||
[b"inline; filename*=utf-8''" + filename + self.test_image.extension],
|
||||
[b"attachment; filename*=utf-8''" + filename + self.test_image.extension],
|
||||
)
|
||||
|
||||
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)
|
||||
|
||||
@ -373,7 +373,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
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"attachment"])
|
||||
|
||||
def test_thumbnail_crop(self) -> None:
|
||||
"""Test that a cropped remote thumbnail is available."""
|
||||
@ -612,7 +612,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
Tests that the `X-Robots-Tag` header is present, which informs web crawlers
|
||||
to not index, archive, or follow links in media.
|
||||
"""
|
||||
channel = self._req(b"inline; filename=out" + self.test_image.extension)
|
||||
channel = self._req(b"attachment; filename=out" + self.test_image.extension)
|
||||
|
||||
headers = channel.headers
|
||||
self.assertEqual(
|
||||
@ -625,7 +625,7 @@ class MediaRepoTests(unittest.HomeserverTestCase):
|
||||
Test that the Cross-Origin-Resource-Policy header is set to "cross-origin"
|
||||
allowing web clients to embed media from the downloads API.
|
||||
"""
|
||||
channel = self._req(b"inline; filename=out" + self.test_image.extension)
|
||||
channel = self._req(b"attachment; filename=out" + self.test_image.extension)
|
||||
|
||||
headers = channel.headers
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user