summary refs log tree commit diff
path: root/synapse
diff options
context:
space:
mode:
authorAndrew Morgan <1342360+anoadragon453@users.noreply.github.com>2021-03-17 16:51:55 +0000
committerGitHub <noreply@github.com>2021-03-17 16:51:55 +0000
commit7b06f85c0e18b62775f12789fdf4adb6a0a47a4b (patch)
treed0cb45f79946c7600b77ee45e7be197950fde319 /synapse
parentFix up types for the typing handler. (#9638) (diff)
downloadsynapse-7b06f85c0e18b62775f12789fdf4adb6a0a47a4b.tar.xz
Ensure we use a copy of the event content dict before modifying it in serialize_event (#9585)
This bug was discovered by DINUM. We were modifying `serialized_event["content"]`, which - if you've got `USE_FROZEN_DICTS` turned on or are [using a third party rules module](https://github.com/matrix-org/synapse/blob/17cd48fe5171d50da4cb59db647b993168e7dfab/synapse/events/third_party_rules.py#L73-L76) - will raise a 500 if you try to a edit a reply to a message.

`serialized_event["content"]` could be set to the edit event's content, instead of a copy of it, which is bad as we attempt to modify it. Instead, we also end up modifying the original event's content. DINUM uses a third party rules module, which meant the event's content got frozen and thus an exception was raised.

To be clear, the problem is not that the event's content was frozen. In fact doing so helped us uncover the fact we weren't copying event content correctly.
Diffstat (limited to 'synapse')
-rw-r--r--synapse/events/utils.py14
1 files changed, 12 insertions, 2 deletions
diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index 5022e0fcb3..0f8a3b5ad8 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -22,6 +22,7 @@ from synapse.api.constants import EventTypes, RelationTypes
 from synapse.api.errors import Codes, SynapseError
 from synapse.api.room_versions import RoomVersion
 from synapse.util.async_helpers import yieldable_gather_results
+from synapse.util.frozenutils import unfreeze
 
 from . import EventBase
 
@@ -402,10 +403,19 @@ class EventClientSerializer:
                 # If there is an edit replace the content, preserving existing
                 # relations.
 
+                # Ensure we take copies of the edit content, otherwise we risk modifying
+                # the original event.
+                edit_content = edit.content.copy()
+
+                # Unfreeze the event content if necessary, so that we may modify it below
+                edit_content = unfreeze(edit_content)
+                serialized_event["content"] = edit_content.get("m.new_content", {})
+
+                # Check for existing relations
                 relations = event.content.get("m.relates_to")
-                serialized_event["content"] = edit.content.get("m.new_content", {})
                 if relations:
-                    serialized_event["content"]["m.relates_to"] = relations
+                    # Keep the relations, ensuring we use a dict copy of the original
+                    serialized_event["content"]["m.relates_to"] = relations.copy()
                 else:
                     serialized_event["content"].pop("m.relates_to", None)