From 94b2195c5b2c0fddb5da026608df114ce3741511 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 1 Apr 2026 16:09:51 +0100 Subject: [PATCH 1/4] Bail out if `admin_unsafely_bypass_quarantine` was used by non-admin Previously the code would send an error back to the client, but would continue executing and fetch the remote media anyhow. This isn't too much of an issue, as remote media must already be cached locally in order to quarantine it in the first place. --- synapse/rest/client/media.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/rest/client/media.py b/synapse/rest/client/media.py index 4db3b015762..15f58acb955 100644 --- a/synapse/rest/client/media.py +++ b/synapse/rest/client/media.py @@ -253,6 +253,7 @@ async def on_GET( ), send_cors=True, ) + return set_cors_headers(request) set_corp_headers(request) From 212b110e0a7dc404f6dfd197c54d26c5aa5028f5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 1 Apr 2026 16:34:09 +0100 Subject: [PATCH 2/4] Add a regression test --- tests/rest/admin/test_admin.py | 44 +++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py index 77d824dcd84..67e82ba84a8 100644 --- a/tests/rest/admin/test_admin.py +++ b/tests/rest/admin/test_admin.py @@ -20,10 +20,12 @@ # import urllib.parse -from typing import cast +from typing import Any, cast +from unittest.mock import Mock from parameterized import parameterized +from twisted.internet.defer import Deferred from twisted.internet.testing import MemoryReactor from twisted.web.resource import Resource @@ -70,6 +72,24 @@ def create_resource_dict(self) -> dict[str, Resource]: resources["/_matrix/media"] = self.hs.get_media_repository_resource() return resources + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + self.fetches: list[tuple[tuple[Any, ...], dict[str, Any]]] = [] + + # A remote fetch of media that was not intentional. + # Used to check that remote media fetches do NOT happen. + def unexpected_remote_fetch(*args: Any, **kwargs: Any) -> Deferred[Any]: + self.fetches.append((args, kwargs)) + return Deferred() + + client = Mock() + client.federation_get_file = unexpected_remote_fetch + client.get_file = unexpected_remote_fetch + + return self.setup_test_homeserver( + clock=clock, + federation_http_client=client, + ) + def _ensure_quarantined( self, user_tok: str, @@ -176,6 +196,28 @@ def test_admin_can_bypass_quarantine(self) -> None: ), ) + def test_non_admin_bypass_does_not_fetch_remote_media(self) -> None: + self.register_user("nonadmin", "pass", admin=False) + non_admin_user_tok = self.login("nonadmin", "pass") + + channel = self.make_request( + "GET", + "/_matrix/client/v1/media/download/example.com/remote_media" + "?admin_unsafely_bypass_quarantine=true", + shorthand=False, + access_token=non_admin_user_tok, + await_result=False, + ) + self.pump() + + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual( + channel.json_body["error"], + "Must be a server admin to bypass quarantine", + ) + # Check that a remote fetch attempt did not occur. + self.assertEqual(self.fetches, []) + @parameterized.expand( [ # Attempt quarantine media APIs as non-admin From 2cadf90228c61f9841971a177d03f0ce9d8a62b5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 1 Apr 2026 16:37:06 +0100 Subject: [PATCH 3/4] newsfile --- changelog.d/19639.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19639.bugfix diff --git a/changelog.d/19639.bugfix b/changelog.d/19639.bugfix new file mode 100644 index 00000000000..2d2928c1eea --- /dev/null +++ b/changelog.d/19639.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.145 where a non-admin could bypass admin checks for downloading remote quarantined media. This relied on the media already being previously present on the homeserver. \ No newline at end of file From ce5d3ea1a27172a8c8a925cea758bf0213e85181 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 17 Apr 2026 14:43:45 +0100 Subject: [PATCH 4/4] Fix `TypeError: 'type' object is not subscriptable` Prevent types from being evaluated at runtime. The alternative to just quoting the type. --- tests/rest/admin/test_admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py index 67e82ba84a8..b77a72ec4af 100644 --- a/tests/rest/admin/test_admin.py +++ b/tests/rest/admin/test_admin.py @@ -18,6 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # +from __future__ import annotations import urllib.parse from typing import Any, cast