From 5bc5d6053f9d741c2fe13550f62b5eb20b1b57bd Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 8 Jul 2019 17:10:11 +0100 Subject: [PATCH] prevent leakage on /aggregations and fix tests --- synapse/rest/client/v1/room.py | 2 +- synapse/rest/client/v2_alpha/relations.py | 20 +++- tests/rest/client/v2_alpha/test_relations.py | 111 ++++++++++++++++--- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 105e68353fc9..aba3dd09b660 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -748,7 +748,7 @@ def on_POST(self, request, room_id, event_id, txn_id=None): relation_ids = relation_chunk.to_dict() for relation_id in relation_ids: - self.event_creation_handler.create_and_send_nonmember_event( + yield self.event_creation_handler.create_and_send_nonmember_event( requester, { "type": EventTypes.Redaction, diff --git a/synapse/rest/client/v2_alpha/relations.py b/synapse/rest/client/v2_alpha/relations.py index 8e362782cc3c..44a022488428 100644 --- a/synapse/rest/client/v2_alpha/relations.py +++ b/synapse/rest/client/v2_alpha/relations.py @@ -147,12 +147,20 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non # This checks that a) the event exists and b) the user is allowed to # view it. - yield self.event_handler.get_event(requester.user, room_id, parent_id) + event = yield self.event_handler.get_event(requester.user, room_id, parent_id) limit = parse_integer(request, "limit", default=5) from_token = parse_string(request, "from") to_token = parse_string(request, "to") + # Check if the event is redacted, and if so return an empty chunk + # list and zero tokens + if "redacted_because" in event.unsigned: + res = { + "chunk": [], + } + defer.returnValue((200, res)) + if from_token: from_token = RelationPaginationToken.from_string(from_token) @@ -222,7 +230,7 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non # This checks that a) the event exists and b) the user is allowed to # view it. - yield self.event_handler.get_event(requester.user, room_id, parent_id) + event = yield self.event_handler.get_event(requester.user, room_id, parent_id) if relation_type not in (RelationTypes.ANNOTATION, None): raise SynapseError(400, "Relation type must be 'annotation'") @@ -231,6 +239,14 @@ def on_GET(self, request, room_id, parent_id, relation_type=None, event_type=Non from_token = parse_string(request, "from") to_token = parse_string(request, "to") + # Check if the event is redacted, and if so return an empty chunk + # list and zero tokens + if "redacted_because" in event.unsigned: + res = { + "chunk": [], + } + defer.returnValue((200, res)) + if from_token: from_token = AggregationPaginationToken.from_string(from_token) diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py index f786ac0111a7..668e41f26be1 100644 --- a/tests/rest/client/v2_alpha/test_relations.py +++ b/tests/rest/client/v2_alpha/test_relations.py @@ -93,7 +93,7 @@ def test_deny_membership(self): def test_deny_double_react(self): """Test that we deny relations on membership events """ - channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="a") self.assertEquals(200, channel.code, channel.json_body) channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") @@ -523,44 +523,127 @@ def test_multi_edit(self): {RelationTypes.REPLACE: {"event_id": edit_event_id}}, ) - def test_aggregation_redaction_redacts_edits(self): + def test_relations_redaction_redacts_edits(self): """Test that edits of an event are redacted when the original event is redacted. - - NOTE: This will delete the event with id self.parent_id and all - associated message edits. Be sure not to put any tests after this - that require those. """ + # Send a new event + res = self.helper.send(self.room, body="Heyo!", tok=self.user_token) + original_event_id = res["event_id"] - # Redact the original + # Add a relation + channel = self._send_relation( + RelationTypes.REPLACE, + "m.room.message", + parent_id=original_event_id, + content={ + "msgtype": "m.text", + "body": "Wibble", + "m.new_content": {"msgtype": "m.text", "body": "First edit"}, + }, + ) + self.assertEquals(200, channel.code, channel.json_body) + + # Check the relation is returned + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/relations/%s/m.replace/m.room.message" + % (self.room, original_event_id), + access_token=self.user_token, + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertIn("chunk", channel.json_body) + self.assertEquals(len(channel.json_body["chunk"]), 1) + + # Redact the original event request, channel = self.make_request( "PUT", "/rooms/%s/redact/%s/%s" - % (self.room, self.parent_id, "test_aggregation_redaction_redacts_edits"), + % (self.room, original_event_id, "test_relations_redaction_redacts_edits"), access_token=self.user_token, + content="{}", ) self.render(request) self.assertEquals(200, channel.code, channel.json_body) - # TODO: Make this request 403 (will need to stick it in the new agg. MSC most likely) # Try to check for remaining m.replace relations request, channel = self.make_request( "GET", "/_matrix/client/unstable/rooms/%s/relations/%s/m.replace/m.room.message" - % (self.room, self.parent_id), + % (self.room, original_event_id), access_token=self.user_token, ) self.render(request) - self.assertEquals(403, channel.code, channel.json_body) + self.assertEquals(200, channel.code, channel.json_body) + + # Check that no relations are returned + self.assertIn("chunk", channel.json_body) + self.assertEquals( + channel.json_body["chunk"], [], + ) + + def test_aggregations_redaction_prevents_access_to_aggregations(self): + """Test that annotations of an event are redacted when the original event + is redacted. + """ + # Send a new event + res = self.helper.send(self.room, body="Hello!", tok=self.user_token) + original_event_id = res["event_id"] + + # Add a relation + channel = self._send_relation( + RelationTypes.ANNOTATION, "m.reaction", key="👍", parent_id=original_event_id, + ) + self.assertEquals(200, channel.code, channel.json_body) + + # Redact the original + request, channel = self.make_request( + "PUT", + "/rooms/%s/redact/%s/%s" + % ( + self.room, + original_event_id, + "test_aggregations_redaction_prevents_access_to_aggregations", + ), + access_token=self.user_token, + content="{}", + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + # Check that aggregations returns zero + request, channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/aggregations/%s/m.annotation/m.reaction" + % (self.room, original_event_id), + access_token=self.user_token, + ) + self.render(request) + self.assertEquals(200, channel.code, channel.json_body) + + self.assertIn("chunk", channel.json_body) + self.assertEquals( + channel.json_body["chunk"], [], + ) + def _send_relation( - self, relation_type, event_type, key=None, content={}, access_token=None + self, + relation_type, + event_type, + key=None, + content={}, + access_token=None, + parent_id=None, ): """Helper function to send a relation pointing at `self.parent_id` Args: relation_type (str): One of `RelationTypes` event_type (str): The type of the event to create + parent_id (str): The event_id this relation relates to. If None, then self.parent_id key (str|None): The aggregation key used for m.annotation relation type. content(dict|None): The content of the created event. @@ -577,10 +660,12 @@ def _send_relation( if key: query = "?key=" + six.moves.urllib.parse.quote_plus(key.encode("utf-8")) + original_id = parent_id if parent_id else self.parent_id + request, channel = self.make_request( "POST", "/_matrix/client/unstable/rooms/%s/send_relation/%s/%s/%s%s" - % (self.room, self.parent_id, relation_type, event_type, query), + % (self.room, original_id, relation_type, event_type, query), json.dumps(content).encode("utf-8"), access_token=access_token, )