From 1000b20577dbfb1967149bbe5f393907892bd43c Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 25 Apr 2024 11:32:37 +0100 Subject: [PATCH] Restrict MSC4108 content-type to text/plain --- rust/src/rendezvous/mod.rs | 11 ++++ tests/rest/client/test_rendezvous.py | 79 +++++++++++++++++++++++----- tests/server.py | 7 ++- tests/unittest.py | 5 ++ 4 files changed, 89 insertions(+), 13 deletions(-) diff --git a/rust/src/rendezvous/mod.rs b/rust/src/rendezvous/mod.rs index 82721d51b70..c0f5d8b6000 100644 --- a/rust/src/rendezvous/mod.rs +++ b/rust/src/rendezvous/mod.rs @@ -77,6 +77,17 @@ impl RendezvousHandler { let content_type: ContentType = headers.typed_get_required()?; + // Content-Type must be text/plain + if content_type != ContentType::text() { + return Err(SynapseError::new( + StatusCode::BAD_REQUEST, + "Content-Type must be text/plain".to_owned(), + "M_INVALID_PARAM", + None, + None, + )); + } + Ok(content_type.into()) } diff --git a/tests/rest/client/test_rendezvous.py b/tests/rest/client/test_rendezvous.py index 1ad56a22859..0ab754a11aa 100644 --- a/tests/rest/client/test_rendezvous.py +++ b/tests/rest/client/test_rendezvous.py @@ -118,8 +118,7 @@ def test_msc4108(self) -> None: "POST", msc4108_endpoint, "foo=bar", - # This sets the content type to application/x-www-form-urlencoded - content_is_form=True, + content_type=b"text/plain", access_token=None, ) self.assertEqual(channel.code, 201) @@ -150,9 +149,7 @@ def test_msc4108(self) -> None: headers = dict(channel.headers.getAllRawHeaders()) self.assertEqual(headers[b"ETag"], [etag]) self.assertIn(b"Expires", headers) - self.assertEqual( - headers[b"Content-Type"], [b"application/x-www-form-urlencoded"] - ) + self.assertEqual(headers[b"Content-Type"], [b"text/plain"]) self.assertEqual(headers[b"Access-Control-Allow-Origin"], [b"*"]) self.assertEqual(headers[b"Access-Control-Expose-Headers"], [b"etag"]) self.assertEqual(headers[b"Cache-Control"], [b"no-store"]) @@ -174,7 +171,7 @@ def test_msc4108(self) -> None: "PUT", session_endpoint, "foo=baz", - content_is_form=True, + content_type=b"text/plain", access_token=None, custom_headers=[("If-Match", etag)], ) @@ -189,7 +186,7 @@ def test_msc4108(self) -> None: "PUT", session_endpoint, "bar=baz", - content_is_form=True, + content_type=b"text/plain", access_token=None, custom_headers=[("If-Match", old_etag)], ) @@ -258,7 +255,7 @@ def test_msc4108_expiration(self) -> None: "POST", msc4108_endpoint, "foo=bar", - content_is_form=True, + content_type=b"text/plain", access_token=None, ) self.assertEqual(channel.code, 201) @@ -311,7 +308,7 @@ def test_msc4108_capacity(self) -> None: "POST", msc4108_endpoint, "foo=bar", - content_is_form=True, + content_type=b"text/plain", access_token=None, ) self.assertEqual(channel.code, 201) @@ -332,7 +329,7 @@ def test_msc4108_capacity(self) -> None: "POST", msc4108_endpoint, "foo=bar", - content_is_form=True, + content_type=b"text/plain", access_token=None, ) self.assertEqual(channel.code, 201) @@ -383,7 +380,7 @@ def test_msc4108_hard_capacity(self) -> None: "POST", msc4108_endpoint, "foo=bar", - content_is_form=True, + content_type=b"text/plain", access_token=None, ) self.assertEqual(channel.code, 201) @@ -406,7 +403,7 @@ def test_msc4108_hard_capacity(self) -> None: "POST", msc4108_endpoint, "foo=bar", - content_is_form=True, + content_type=b"text/plain", access_token=None, ) self.assertEqual(channel.code, 201) @@ -419,3 +416,61 @@ def test_msc4108_hard_capacity(self) -> None: ) self.assertEqual(channel.code, 404) + + @unittest.skip_unless(HAS_AUTHLIB, "requires authlib") + @override_config( + { + "disable_registration": True, + "experimental_features": { + "msc4108_enabled": True, + "msc3861": { + "enabled": True, + "issuer": "https://issuer", + "client_id": "client_id", + "client_auth_method": "client_secret_post", + "client_secret": "client_secret", + "admin_token": "admin_token_value", + }, + }, + } + ) + def test_msc4108_content_type(self) -> None: + """ + Test that the content-type is restricted to text/plain. + """ + # We cannot post invalid content-type arbitrary data to the endpoint + channel = self.make_request( + "POST", + msc4108_endpoint, + "foo=bar", + content_is_form=True, + access_token=None, + ) + self.assertEqual(channel.code, 400) + self.assertEqual(channel.json_body["errcode"], "M_INVALID_PARAM") + + # Make a valid request + channel = self.make_request( + "POST", + msc4108_endpoint, + "foo=bar", + content_type=b"text/plain", + access_token=None, + ) + self.assertEqual(channel.code, 201) + url = urlparse(channel.json_body["url"]) + session_endpoint = url.path + headers = dict(channel.headers.getAllRawHeaders()) + etag = headers[b"ETag"][0] + + # We can't update the data with invalid content-type + channel = self.make_request( + "PUT", + session_endpoint, + "foo=baz", + content_is_form=True, + access_token=None, + custom_headers=[("If-Match", etag)], + ) + self.assertEqual(channel.code, 400) + self.assertEqual(channel.json_body["errcode"], "M_INVALID_PARAM") diff --git a/tests/server.py b/tests/server.py index 4aaa91e956a..434be3d22c6 100644 --- a/tests/server.py +++ b/tests/server.py @@ -351,6 +351,7 @@ def make_request( request: Type[Request] = SynapseRequest, shorthand: bool = True, federation_auth_origin: Optional[bytes] = None, + content_type: Optional[bytes] = None, content_is_form: bool = False, await_result: bool = True, custom_headers: Optional[Iterable[CustomHeaderType]] = None, @@ -373,6 +374,8 @@ def make_request( with the usual REST API path, if it doesn't contain it. federation_auth_origin: if set to not-None, we will add a fake Authorization header pretenting to be the given server name. + content_type: The content-type to use for the request. If not set then will default to + application/json unless content_is_form is true. content_is_form: Whether the content is URL encoded form data. Adds the 'Content-Type': 'application/x-www-form-urlencoded' header. await_result: whether to wait for the request to complete rendering. If true, @@ -436,7 +439,9 @@ def make_request( ) if content: - if content_is_form: + if content_type is not None: + req.requestHeaders.addRawHeader(b"Content-Type", content_type) + elif content_is_form: req.requestHeaders.addRawHeader( b"Content-Type", b"application/x-www-form-urlencoded" ) diff --git a/tests/unittest.py b/tests/unittest.py index 6fe0cd4a2dc..e6aad9ed40b 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -523,6 +523,7 @@ def make_request( request: Type[Request] = SynapseRequest, shorthand: bool = True, federation_auth_origin: Optional[bytes] = None, + content_type: Optional[bytes] = None, content_is_form: bool = False, await_result: bool = True, custom_headers: Optional[Iterable[CustomHeaderType]] = None, @@ -541,6 +542,9 @@ def make_request( with the usual REST API path, if it doesn't contain it. federation_auth_origin: if set to not-None, we will add a fake Authorization header pretenting to be the given server name. + + content_type: The content-type to use for the request. If not set then will default to + application/json unless content_is_form is true. content_is_form: Whether the content is URL encoded form data. Adds the 'Content-Type': 'application/x-www-form-urlencoded' header. @@ -566,6 +570,7 @@ def make_request( request, shorthand, federation_auth_origin, + content_type, content_is_form, await_result, custom_headers,