From 827f198177c4cf547b9d2d1eed41411e945fc199 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 21 Mar 2023 09:13:43 +0000
Subject: [PATCH] Fix error when sending message into deleted room. (#15235)

When a room is deleted in Synapse we remove the event forward
extremities in the room, so if (say a bot) tries to send a message into
the room we error out due to not being able to calculate prev events for
the new event *before* we check if the sender is in the room.

Fixes #8094
---
 changelog.d/15235.bugfix      |  1 +
 synapse/handlers/message.py   | 17 +++++++++++++++--
 tests/rest/admin/test_room.py | 15 +++++++++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/15235.bugfix

diff --git a/changelog.d/15235.bugfix b/changelog.d/15235.bugfix
new file mode 100644
index 000000000..e6a6bb1b9
--- /dev/null
+++ b/changelog.d/15235.bugfix
@@ -0,0 +1 @@
+Fix long-standing error when sending message into deleted room.
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index da129ec16..4c75433a6 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -987,10 +987,11 @@ class EventCreationHandler:
         # a situation where event persistence can't keep up, causing
         # extremities to pile up, which in turn leads to state resolution
         # taking longer.
-        async with self.limiter.queue(event_dict["room_id"]):
+        room_id = event_dict["room_id"]
+        async with self.limiter.queue(room_id):
             if txn_id:
                 event = await self.get_event_from_transaction(
-                    requester, txn_id, event_dict["room_id"]
+                    requester, txn_id, room_id
                 )
                 if event:
                     # we know it was persisted, so must have a stream ordering
@@ -1000,6 +1001,18 @@ class EventCreationHandler:
                         event.internal_metadata.stream_ordering,
                     )
 
+        # If we don't have any prev event IDs specified then we need to
+        # check that the host is in the room (as otherwise populating the
+        # prev events will fail), at which point we may as well check the
+        # local user is in the room.
+        if not prev_event_ids:
+            user_id = requester.user.to_string()
+            is_user_in_room = await self.store.check_local_user_in_room(
+                user_id, room_id
+            )
+            if not is_user_in_room:
+                raise AuthError(403, f"User {user_id} not in room {room_id}")
+
         # Try several times, it could fail with PartialStateConflictError
         # in handle_new_client_event, cf comment in except block.
         max_retries = 5
diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py
index 9dbb77867..eb50086c5 100644
--- a/tests/rest/admin/test_room.py
+++ b/tests/rest/admin/test_room.py
@@ -402,6 +402,21 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
         # Assert we can no longer peek into the room
         self._assert_peek(self.room_id, expect_code=403)
 
+    def test_room_delete_send(self) -> None:
+        """Test that sending into a deleted room returns a 403"""
+        channel = self.make_request(
+            "DELETE",
+            self.url,
+            content={},
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(200, channel.code, msg=channel.json_body)
+
+        self.helper.send(
+            self.room_id, "test message", expect_code=403, tok=self.other_user_tok
+        )
+
     def _is_blocked(self, room_id: str, expect: bool = True) -> None:
         """Assert that the room is blocked or not"""
         d = self.store.is_room_blocked(room_id)