Synapse vulnerable to denial of service (DoS) due to incorrect application of event authorization rules

@@ -0,0 +1 @@
+Fix some inconsistencies in the event authentication code.

@@ -150,14 +150,15 @@ async def check_state_independent_auth_rules(
         # 1.5 Otherwise, allow
         return
 
-    # Check the auth events.
+    # 2. Reject if event has auth_events that: ...
     auth_events = await store.get_events(
         event.auth_event_ids(),
         redact_behaviour=EventRedactBehaviour.as_is,
         allow_rejected=True,
     )
     room_id = event.room_id
     auth_dict: MutableStateMap[str] = {}
+    expected_auth_types = auth_types_for_event(event.room_version, event)
     for auth_event_id in event.auth_event_ids():
         auth_event = auth_events.get(auth_event_id)
 
@@ -179,6 +180,24 @@ async def check_state_independent_auth_rules(
                 % (event.event_id, room_id, auth_event_id, auth_event.room_id),
             )
 
+        k = (auth_event.type, auth_event.state_key)
+
+        # 2.1 ... have duplicate entries for a given type and state_key pair
+        if k in auth_dict:
+            raise AuthError(
+                403,
+                f"Event {event.event_id} has duplicate auth_events for {k}: {auth_dict[k]} and {auth_event_id}",
+            )
+
+        # 2.2 ... have entries whose type and state_key don’t match those specified by
+        #   the auth events selection algorithm described in the server
+        #   specification.
+        if k not in expected_auth_types:
+            raise AuthError(
+                403,
+                f"Event {event.event_id} has unexpected auth_event for {k}: {auth_event_id}",
+            )
+
         # We also need to check that the auth event itself is not rejected.
         if auth_event.rejected_reason:
             raise AuthError(
@@ -187,7 +206,7 @@ async def check_state_independent_auth_rules(
                 % (event.event_id, auth_event.event_id),
             )
 
-        auth_dict[(auth_event.type, auth_event.state_key)] = auth_event_id
+        auth_dict[k] = auth_event_id
 
     # 3. If event does not have a m.room.create in its auth_events, reject.
     creation_event = auth_dict.get((EventTypes.Create, ""), None)

@@ -98,7 +98,6 @@ def _test_process_pulled_event_with_missing_state(
         auth_event_ids = [
             initial_state_map[("m.room.create", "")],
             initial_state_map[("m.room.power_levels", "")],
-            initial_state_map[("m.room.join_rules", "")],
             member_event.event_id,
         ]
 

@@ -225,9 +225,10 @@ def test_backfill_with_many_backward_extremities(self) -> None:
 
         # we need a user on the remote server to be a member, so that we can send
         # extremity-causing events.
+        remote_server_user_id = f"@user:{self.OTHER_SERVER_NAME}"
         self.get_success(
             event_injection.inject_member_event(
-                self.hs, room_id, f"@user:{self.OTHER_SERVER_NAME}", "join"
+                self.hs, room_id, remote_server_user_id, "join"
             )
         )
 
@@ -247,6 +248,12 @@ def test_backfill_with_many_backward_extremities(self) -> None:
         # create more than is 5 which corresponds to the number of backward
         # extremities we slice off in `_maybe_backfill_inner`
         federation_event_handler = self.hs.get_federation_event_handler()
+        auth_events = [
+            ev
+            for ev in current_state
+            if (ev.type, ev.state_key)
+            in {("m.room.create", ""), ("m.room.member", remote_server_user_id)}
+        ]
         for _ in range(0, 8):
             event = make_event_from_dict(
                 self.add_hashes_and_signatures(
@@ -258,15 +265,14 @@ def test_backfill_with_many_backward_extremities(self) -> None:
                             "body": "message connected to fake event",
                         },
                         "room_id": room_id,
-                        "sender": f"@user:{self.OTHER_SERVER_NAME}",
+                        "sender": remote_server_user_id,
                         "prev_events": [
                             ev1.event_id,
                             # We're creating an backward extremity each time thanks
                             # to this fake event
                             generate_fake_event_id(),
                         ],
-                        # lazy: *everything* is an auth event
-                        "auth_events": [ev.event_id for ev in current_state],
+                        "auth_events": [ev.event_id for ev in auth_events],
                         "depth": ev1.depth + 1,
                     },
                     room_version,

@@ -150,6 +150,92 @@ def test_create_event_with_prev_events(self):
                 event_auth.check_state_independent_auth_rules(event_store, bad_event)
             )
 
+    def test_duplicate_auth_events(self):
+        """Events with duplicate auth_events should be rejected
+
+        https://spec.matrix.org/v1.3/rooms/v9/#authorization-rules
+        2. Reject if event has auth_events that:
+            1. have duplicate entries for a given type and state_key pair
+        """
+        creator = "@creator:example.com"
+
+        create_event = _create_event(RoomVersions.V9, creator)
+        join_event1 = _join_event(RoomVersions.V9, creator)
+        pl_event = _power_levels_event(
+            RoomVersions.V9,
+            creator,
+            {"state_default": 30, "users": {"creator": 100}},
+        )
+
+        # create a second join event, so that we can make a duplicate
+        join_event2 = _join_event(RoomVersions.V9, creator)
+
+        event_store = _StubEventSourceStore()
+        event_store.add_events([create_event, join_event1, join_event2, pl_event])
+
+        good_event = _random_state_event(
+            RoomVersions.V9, creator, [create_event, join_event2, pl_event]
+        )
+        bad_event = _random_state_event(
+            RoomVersions.V9, creator, [create_event, join_event1, join_event2, pl_event]
+        )
+        # a variation: two instances of the *same* event
+        bad_event2 = _random_state_event(
+            RoomVersions.V9, creator, [create_event, join_event2, join_event2, pl_event]
+        )
+
+        get_awaitable_result(
+            event_auth.check_state_independent_auth_rules(event_store, good_event)
+        )
+        with self.assertRaises(AuthError):
+            get_awaitable_result(
+                event_auth.check_state_independent_auth_rules(event_store, bad_event)
+            )
+        with self.assertRaises(AuthError):
+            get_awaitable_result(
+                event_auth.check_state_independent_auth_rules(event_store, bad_event2)
+            )
+
+    def test_unexpected_auth_events(self):
+        """Events with excess auth_events should be rejected
+
+        https://spec.matrix.org/v1.3/rooms/v9/#authorization-rules
+        2. Reject if event has auth_events that:
+           2. have entries whose type and state_key don’t match those specified by the
+              auth events selection algorithm described in the server specification.
+        """
+        creator = "@creator:example.com"
+
+        create_event = _create_event(RoomVersions.V9, creator)
+        join_event = _join_event(RoomVersions.V9, creator)
+        pl_event = _power_levels_event(
+            RoomVersions.V9,
+            creator,
+            {"state_default": 30, "users": {"creator": 100}},
+        )
+        join_rules_event = _join_rules_event(RoomVersions.V9, creator, "public")
+
+        event_store = _StubEventSourceStore()
+        event_store.add_events([create_event, join_event, pl_event, join_rules_event])
+
+        good_event = _random_state_event(
+            RoomVersions.V9, creator, [create_event, join_event, pl_event]
+        )
+        # join rules should *not* be included in the auth events.
+        bad_event = _random_state_event(
+            RoomVersions.V9,
+            creator,
+            [create_event, join_event, pl_event, join_rules_event],
+        )
+
+        get_awaitable_result(
+            event_auth.check_state_independent_auth_rules(event_store, good_event)
+        )
+        with self.assertRaises(AuthError):
+            get_awaitable_result(
+                event_auth.check_state_independent_auth_rules(event_store, bad_event)
+            )
+
     def test_random_users_cannot_send_state_before_first_pl(self):
         """
         Check that, before the first PL lands, the creator is the only user

@@ -0,0 +1 @@
+Fix some inconsistencies in the event authentication code.

@@ -141,6 +141,15 @@ async def check_state_independent_auth_rules(
     Raises:
         AuthError if the checks fail
     """
+    # Implementation of https://spec.matrix.org/v1.2/rooms/v9/#authorization-rules
+
+    # 1. If type is m.room.create:
+    if event.type == EventTypes.Create:
+        _check_create(event)
+
+        # 1.5 Otherwise, allow
+        return
+
     # Check the auth events.
     auth_events = await store.get_events(
         event.auth_event_ids(),
@@ -180,29 +189,6 @@ async def check_state_independent_auth_rules(
 
         auth_dict[(auth_event.type, auth_event.state_key)] = auth_event_id
 
-    # Implementation of https://matrix.org/docs/spec/rooms/v1#authorization-rules
-    #
-    # 1. If type is m.room.create:
-    if event.type == EventTypes.Create:
-        # 1b. If the domain of the room_id does not match the domain of the sender,
-        # reject.
-        sender_domain = get_domain_from_id(event.sender)
-        room_id_domain = get_domain_from_id(event.room_id)
-        if room_id_domain != sender_domain:
-            raise AuthError(
-                403, "Creation event's room_id domain does not match sender's"
-            )
-
-        # 1c. If content.room_version is present and is not a recognised version, reject
-        room_version_prop = event.content.get("room_version", "1")
-        if room_version_prop not in KNOWN_ROOM_VERSIONS:
-            raise AuthError(
-                403,
-                "room appears to have unsupported version %s" % (room_version_prop,),
-            )
-
-        return
-
     # 3. If event does not have a m.room.create in its auth_events, reject.
     creation_event = auth_dict.get((EventTypes.Create, ""), None)
     if not creation_event:
@@ -324,6 +310,41 @@ def _check_size_limits(event: "EventBase") -> None:
         raise EventSizeError("event too large")
 
 
+def _check_create(event: "EventBase") -> None:
+    """Implementation of the auth rules for m.room.create events
+
+    Args:
+        event: The `m.room.create` event to be checked
+
+    Raises:
+        AuthError if the event does not pass the auth rules
+    """
+    assert event.type == EventTypes.Create
+
+    #  1.1 If it has any previous events, reject.
+    if event.prev_event_ids():
+        raise AuthError(403, "Create event has prev events")
+
+    # 1.2 If the domain of the room_id does not match the domain of the sender,
+    # reject.
+    sender_domain = get_domain_from_id(event.sender)
+    room_id_domain = get_domain_from_id(event.room_id)
+    if room_id_domain != sender_domain:
+        raise AuthError(403, "Creation event's room_id domain does not match sender's")
+
+    # 1.3 If content.room_version is present and is not a recognised version, reject
+    room_version_prop = event.content.get("room_version", "1")
+    if room_version_prop not in KNOWN_ROOM_VERSIONS:
+        raise AuthError(
+            403,
+            "room appears to have unsupported version %s" % (room_version_prop,),
+        )
+
+    # 1.4 If content has no creator field, reject.
+    if EventContentFields.ROOM_CREATOR not in event.content:
+        raise AuthError(403, "Create event lacks a 'creator' property")
+
+
 def _can_federate(event: "EventBase", auth_events: StateMap["EventBase"]) -> bool:
     creation_event = auth_events.get((EventTypes.Create, ""))
     # There should always be a creation event, but if not don't federate.

@@ -109,6 +109,47 @@ def test_rejected_auth_events(self):
                 )
             )
 
+    def test_create_event_with_prev_events(self):
+        """A create event with prev_events should be rejected
+
+        https://spec.matrix.org/v1.3/rooms/v9/#authorization-rules
+        1: If type is m.room.create:
+            1. If it has any previous events, reject.
+        """
+        creator = f"@creator:{TEST_DOMAIN}"
+
+        # we make both a good event and a bad event, to check that we are rejecting
+        # the bad event for the reason we think we are.
+        good_event = make_event_from_dict(
+            {
+                "room_id": TEST_ROOM_ID,
+                "type": "m.room.create",
+                "state_key": "",
+                "sender": creator,
+                "content": {
+                    "creator": creator,
+                    "room_version": RoomVersions.V9.identifier,
+                },
+                "auth_events": [],
+                "prev_events": [],
+            },
+            room_version=RoomVersions.V9,
+        )
+        bad_event = make_event_from_dict(
+            {**good_event.get_dict(), "prev_events": ["$fakeevent"]},
+            room_version=RoomVersions.V9,
+        )
+
+        event_store = _StubEventSourceStore()
+
+        get_awaitable_result(
+            event_auth.check_state_independent_auth_rules(event_store, good_event)
+        )
+        with self.assertRaises(AuthError):
+            get_awaitable_result(
+                event_auth.check_state_independent_auth_rules(event_store, bad_event)
+            )
+
     def test_random_users_cannot_send_state_before_first_pl(self):
         """
         Check that, before the first PL lands, the creator is the only user
@@ -564,8 +605,8 @@ def test_join_rules_msc3083_restricted(self) -> None:
 
 
 # helpers for making events
-
-TEST_ROOM_ID = "!test:room"
+TEST_DOMAIN = "example.com"
+TEST_ROOM_ID = f"!test_room:{TEST_DOMAIN}"
 
 
 def _create_event(