From 3862d25db65a064aa3332b9ceea6d527afb1528d Mon Sep 17 00:00:00 2001 From: santhoshivan23 Date: Sun, 19 Jun 2022 11:00:15 +0530 Subject: [PATCH 1/5] patch: validate room alias prior to calling internal functions Signed-off-by: santhoshivan23 --- synapse/rest/client/directory.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/rest/client/directory.py b/synapse/rest/client/directory.py index 9639d4fe2c8c..eb2381892d89 100644 --- a/synapse/rest/client/directory.py +++ b/synapse/rest/client/directory.py @@ -46,6 +46,8 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() async def on_GET(self, request: Request, room_alias: str) -> Tuple[int, JsonDict]: + if not RoomAlias.is_valid(room_alias): + raise SynapseError(400, 'Room alias invalid', errcode=Codes.BAD_JSON) room_alias_obj = RoomAlias.from_string(room_alias) res = await self.directory_handler.get_association(room_alias_obj) @@ -55,6 +57,8 @@ async def on_GET(self, request: Request, room_alias: str) -> Tuple[int, JsonDict async def on_PUT( self, request: SynapseRequest, room_alias: str ) -> Tuple[int, JsonDict]: + if not RoomAlias.is_valid(room_alias): + raise SynapseError(400, 'Room alias invalid', errcode=Codes.BAD_JSON) room_alias_obj = RoomAlias.from_string(room_alias) content = parse_json_object_from_request(request) @@ -89,6 +93,8 @@ async def on_PUT( async def on_DELETE( self, request: SynapseRequest, room_alias: str ) -> Tuple[int, JsonDict]: + if not RoomAlias.is_valid(room_alias): + raise SynapseError(400, 'Room alias invalid', errcode=Codes.BAD_JSON) room_alias_obj = RoomAlias.from_string(room_alias) requester = await self.auth.get_user_by_req(request) From 2ab85101cb1f6a535e29aeae769bc2ee6f02b980 Mon Sep 17 00:00:00 2001 From: santhoshivan23 Date: Sun, 19 Jun 2022 12:34:59 +0530 Subject: [PATCH 2/5] add changelog entry for PR #13106 Signed-off-by: santhoshivan23 --- changelog.d/13106.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13106.bugfix diff --git a/changelog.d/13106.bugfix b/changelog.d/13106.bugfix new file mode 100644 index 000000000000..6f5f25a08e24 --- /dev/null +++ b/changelog.d/13106.bugfix @@ -0,0 +1 @@ +Validates room alias before calling internal functions \ No newline at end of file From 2173f943855f248d027e58f56ad2afbadd30bd7c Mon Sep 17 00:00:00 2001 From: santhoshivan23 Date: Sun, 19 Jun 2022 19:21:32 +0530 Subject: [PATCH 3/5] linted rest/client/directory.py Signed-off-by: santhoshivan23 --- synapse/rest/client/directory.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/rest/client/directory.py b/synapse/rest/client/directory.py index eb2381892d89..abfdaf909b47 100644 --- a/synapse/rest/client/directory.py +++ b/synapse/rest/client/directory.py @@ -47,7 +47,7 @@ def __init__(self, hs: "HomeServer"): async def on_GET(self, request: Request, room_alias: str) -> Tuple[int, JsonDict]: if not RoomAlias.is_valid(room_alias): - raise SynapseError(400, 'Room alias invalid', errcode=Codes.BAD_JSON) + raise SynapseError(400, "Room alias invalid", errcode=Codes.BAD_JSON) room_alias_obj = RoomAlias.from_string(room_alias) res = await self.directory_handler.get_association(room_alias_obj) @@ -58,7 +58,7 @@ async def on_PUT( self, request: SynapseRequest, room_alias: str ) -> Tuple[int, JsonDict]: if not RoomAlias.is_valid(room_alias): - raise SynapseError(400, 'Room alias invalid', errcode=Codes.BAD_JSON) + raise SynapseError(400, "Room alias invalid", errcode=Codes.BAD_JSON) room_alias_obj = RoomAlias.from_string(room_alias) content = parse_json_object_from_request(request) @@ -94,7 +94,7 @@ async def on_DELETE( self, request: SynapseRequest, room_alias: str ) -> Tuple[int, JsonDict]: if not RoomAlias.is_valid(room_alias): - raise SynapseError(400, 'Room alias invalid', errcode=Codes.BAD_JSON) + raise SynapseError(400, "Room alias invalid", errcode=Codes.BAD_JSON) room_alias_obj = RoomAlias.from_string(room_alias) requester = await self.auth.get_user_by_req(request) From b1b37481c0a4e14ee77e8db3a42b8266fc3f03cb Mon Sep 17 00:00:00 2001 From: santhoshivan23 Date: Mon, 20 Jun 2022 09:42:50 +0530 Subject: [PATCH 4/5] format changelog entry --- changelog.d/13106.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/13106.bugfix b/changelog.d/13106.bugfix index 6f5f25a08e24..0dc16bad08d2 100644 --- a/changelog.d/13106.bugfix +++ b/changelog.d/13106.bugfix @@ -1 +1 @@ -Validates room alias before calling internal functions \ No newline at end of file +Fix a long-standing bug where room directory requests would cause an internal server error if given a malformed room alias. \ No newline at end of file From f776fba878f6afd267334aa9ba99e32c3015e472 Mon Sep 17 00:00:00 2001 From: santhoshivan23 Date: Mon, 20 Jun 2022 12:19:28 +0530 Subject: [PATCH 5/5] Rectifies error codes for invalid params and test cases for invalid room aliases --- synapse/rest/client/directory.py | 6 +++--- tests/rest/client/test_directory.py | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/synapse/rest/client/directory.py b/synapse/rest/client/directory.py index abfdaf909b47..d6c89cb16297 100644 --- a/synapse/rest/client/directory.py +++ b/synapse/rest/client/directory.py @@ -47,7 +47,7 @@ def __init__(self, hs: "HomeServer"): async def on_GET(self, request: Request, room_alias: str) -> Tuple[int, JsonDict]: if not RoomAlias.is_valid(room_alias): - raise SynapseError(400, "Room alias invalid", errcode=Codes.BAD_JSON) + raise SynapseError(400, "Room alias invalid", errcode=Codes.INVALID_PARAM) room_alias_obj = RoomAlias.from_string(room_alias) res = await self.directory_handler.get_association(room_alias_obj) @@ -58,7 +58,7 @@ async def on_PUT( self, request: SynapseRequest, room_alias: str ) -> Tuple[int, JsonDict]: if not RoomAlias.is_valid(room_alias): - raise SynapseError(400, "Room alias invalid", errcode=Codes.BAD_JSON) + raise SynapseError(400, "Room alias invalid", errcode=Codes.INVALID_PARAM) room_alias_obj = RoomAlias.from_string(room_alias) content = parse_json_object_from_request(request) @@ -94,7 +94,7 @@ async def on_DELETE( self, request: SynapseRequest, room_alias: str ) -> Tuple[int, JsonDict]: if not RoomAlias.is_valid(room_alias): - raise SynapseError(400, "Room alias invalid", errcode=Codes.BAD_JSON) + raise SynapseError(400, "Room alias invalid", errcode=Codes.INVALID_PARAM) room_alias_obj = RoomAlias.from_string(room_alias) requester = await self.auth.get_user_by_req(request) diff --git a/tests/rest/client/test_directory.py b/tests/rest/client/test_directory.py index 67473a68d71a..16e7ef41bc15 100644 --- a/tests/rest/client/test_directory.py +++ b/tests/rest/client/test_directory.py @@ -215,6 +215,19 @@ def set_alias_via_directory( self.assertEqual(channel.code, expected_code, channel.result) return alias + def test_invalid_alias(self) -> None: + alias = "#potato" + channel = self.make_request( + "GET", + f"/_matrix/client/r0/directory/room/{alias}", + access_token=self.user_tok, + ) + self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result) + self.assertIn("error", channel.json_body, channel.json_body) + self.assertEqual( + channel.json_body["errcode"], "M_INVALID_PARAM", channel.json_body + ) + def random_alias(self, length: int) -> str: return RoomAlias(random_string(length), self.hs.hostname).to_string()