Additional validation of receipts (#16327)

Reject invalid receipts with a reasonable error message &
expands tests for receipts.

3 files changed, 26 insertions, 4 deletions

diff --git a/synapse/handlers/receipts.py b/synapse/handlers/receipts.py
index 2bacdebfb5..c7edada353 100644
--- a/synapse/handlers/receipts.py
+++ b/synapse/handlers/receipts.py
@@ -37,6 +37,8 @@ class ReceiptsHandler:
         self.server_name = hs.config.server.server_name
         self.store = hs.get_datastores().main
         self.event_auth_handler = hs.get_event_auth_handler()
+        self.event_handler = hs.get_event_handler()
+        self._storage_controllers = hs.get_storage_controllers()
 
         self.hs = hs
 
@@ -81,6 +83,20 @@ class ReceiptsHandler:
                 )
                 continue
 
+            # Let's check that the origin server is in the room before accepting the receipt.
+            # We don't want to block waiting on a partial state so take an
+            # approximation if needed.
+            domains = await self._storage_controllers.state.get_current_hosts_in_room_or_partial_state_approximation(
+                room_id
+            )
+            if origin not in domains:
+                logger.info(
+                    "Ignoring receipt for room %r from server %s as they're not in the room",
+                    room_id,
+                    origin,
+                )
+                continue
+
             for receipt_type, users in room_values.items():
                 for user_id, user_values in users.items():
                     if get_domain_from_id(user_id) != origin:
@@ -158,17 +174,23 @@ class ReceiptsHandler:
         self,
         room_id: str,
         receipt_type: str,
-        user_id: str,
+        user_id: UserID,
         event_id: str,
         thread_id: Optional[str],
     ) -> None:
         """Called when a client tells us a local user has read up to the given
         event_id in the room.
         """
+
+        # Ensure the room/event exists, this will raise an error if the user
+        # cannot view the event.
+        if not await self.event_handler.get_event(user_id, room_id, event_id):
+            return
+
         receipt = ReadReceipt(
             room_id=room_id,
             receipt_type=receipt_type,
-            user_id=user_id,
+            user_id=user_id.to_string(),
             event_ids=[event_id],
             thread_id=thread_id,
             data={"ts": int(self.clock.time_msec())},
diff --git a/synapse/rest/client/read_marker.py b/synapse/rest/client/read_marker.py
index 1707e51972..15e4d56cdb 100644
--- a/synapse/rest/client/read_marker.py
+++ b/synapse/rest/client/read_marker.py
@@ -84,7 +84,7 @@ class ReadMarkerRestServlet(RestServlet):
                 await self.receipts_handler.received_client_receipt(
                     room_id,
                     receipt_type,
-                    user_id=requester.user.to_string(),
+                    user_id=requester.user,
                     event_id=event_id,
                     # Setting the thread ID is not possible with the /read_markers endpoint.
                     thread_id=None,
diff --git a/synapse/rest/client/receipts.py b/synapse/rest/client/receipts.py
index 869a374459..814d075faf 100644
--- a/synapse/rest/client/receipts.py
+++ b/synapse/rest/client/receipts.py
@@ -108,7 +108,7 @@ class ReceiptRestServlet(RestServlet):
             await self.receipts_handler.received_client_receipt(
                 room_id,
                 receipt_type,
-                user_id=requester.user.to_string(),
+                user_id=requester.user,
                 event_id=event_id,
                 thread_id=thread_id,
             )