From e1f4c83f41bf6f06bef3d160eb94eacabe59eff1 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Thu, 5 Dec 2019 14:14:45 +0000
Subject: [PATCH] Sanity-check the rooms of auth events before pulling them in.
 (#6472)

---
 changelog.d/6472.bugfix        |  1 +
 synapse/handlers/federation.py | 34 +++++++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 9 deletions(-)
 create mode 100644 changelog.d/6472.bugfix

diff --git a/changelog.d/6472.bugfix b/changelog.d/6472.bugfix
new file mode 100644
index 000000000..598efb79f
--- /dev/null
+++ b/changelog.d/6472.bugfix
@@ -0,0 +1 @@
+Improve sanity-checking when receiving events over federation.
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 7784b80b7..f5d04cdf9 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -2195,21 +2195,37 @@ class FederationHandler(BaseHandler):
             different_auth,
         )
 
-        # now we state-resolve between our own idea of the auth events, and the remote's
-        # idea of them.
-
-        room_version = yield self.store.get_room_version(event.room_id)
-
         # XXX: currently this checks for redactions but I'm not convinced that is
         # necessary?
         different_events = yield self.store.get_events_as_list(different_auth)
 
-        local_view = dict(auth_events)
-        remote_view = dict(auth_events)
-        remote_view.update({(d.type, d.state_key): d for d in different_events})
+        for d in different_events:
+            if d.room_id != event.room_id:
+                logger.warning(
+                    "Event %s refers to auth_event %s which is in a different room",
+                    event.event_id,
+                    d.event_id,
+                )
 
+                # don't attempt to resolve the claimed auth events against our own
+                # in this case: just use our own auth events.
+                #
+                # XXX: should we reject the event in this case? It feels like we should,
+                # but then shouldn't we also do so if we've failed to fetch any of the
+                # auth events?
+                return context
+
+        # now we state-resolve between our own idea of the auth events, and the remote's
+        # idea of them.
+
+        local_state = auth_events.values()
+        remote_auth_events = dict(auth_events)
+        remote_auth_events.update({(d.type, d.state_key): d for d in different_events})
+        remote_state = remote_auth_events.values()
+
+        room_version = yield self.store.get_room_version(event.room_id)
         new_state = yield self.state_handler.resolve_events(
-            room_version, [list(local_view.values()), list(remote_view.values())], event
+            room_version, (local_state, remote_state), event
         )
 
         logger.info(