From e5731aafeffb64771dc985a5177afae286b8dea3 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Fri, 3 Dec 2021 12:45:00 -0800
Subject: [PATCH 01/19] add some tests to verify we are stripping unauthorized
 fields out of unsigned

---
 tests/test_federation.py | 107 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/tests/test_federation.py b/tests/test_federation.py
index 3eef1c4c05c8..353b0b544792 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -276,3 +276,110 @@ def test_cross_signing_keys_retry(self):
             "ed25519:" + remote_self_signing_key in self_signing_key["keys"].keys(),
         )
         self.assertTrue(remote_self_signing_key in self_signing_key["keys"].values())
+
+
+class StripEventsTestCase(MessageAcceptTests):
+    def setUp(self):
+        super().setUp()
+
+    def strip_unauthorized_unsigned_values(self):
+        most_recent = self.get_success(
+            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
+        )[0]
+        federation_event_handler = self.homeserver.get_federation_event_handler()
+        join_event1 = make_event_from_dict(
+            {
+                "room_id": self.room_id,
+                "sender": "@baduser:test.serv",
+                "state_key": "@baduser:test.serv",
+                "event_id": "$join1:test.serv",
+                "depth": 1000,
+                "origin_server_ts": 1,
+                "type": "m.room.member",
+                "origin": "test.servx",
+                "content": {"membership": "join"},
+                "auth_events": [],
+                "prev_state": [(most_recent, {})],
+                "prev_events": [(most_recent, {})],
+                "unsigned": {"malicious garbage": "hackz", "more warez": "more hackz"},
+            }
+        )
+        self.get_success(
+            federation_event_handler.on_receive_pdu("test.serv", join_event1)
+        )
+
+        event = self.get_success(self.store.get_event("$join1:test.serv"))
+        event_dict = event.get_dict()
+        self.assertTrue("hackz" not in event_dict["unsigned"])
+
+    def strip_event_maintains_allowed_fields(self):
+        most_recent = self.get_success(
+            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
+        )[0]
+        federation_event_handler = self.homeserver.get_federation_event_handler()
+        join_event2 = make_event_from_dict(
+            {
+                "room_id": self.room_id,
+                "sender": "@baduser:test.serv",
+                "state_key": "@baduser:test.serv",
+                "event_id": "$join2:test.serv",
+                "depth": 1000,
+                "origin_server_ts": 1,
+                "type": "m.room.member",
+                "origin": "test.servx",
+                "content": {"membership": "join"},
+                "auth_events": [],
+                "prev_state": [(most_recent, {})],
+                "prev_events": [(most_recent, {})],
+                "unsigned": {
+                    "malicious garbage": "hackz",
+                    "more warez": "more hackz",
+                    "age": 14,
+                    "invite_room_state": [],
+                },
+            }
+        )
+        self.get_success(
+            federation_event_handler.on_receive_pdu("test.serv", join_event2)
+        )
+
+        event = self.get_success(self.store.get_event("$join2:test.serv"))
+        event_dict = event.get_dict()
+        self.assertTrue("age" in event_dict["unsigned"])
+        self.assertTrue("invite_room_state" in event_dict["unsigned"])
+
+    def strip_event_removes_fields_based_on_event_type(self):
+        most_recent = self.get_success(
+            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
+        )[0]
+        federation_event_handler = self.homeserver.get_federation_event_handler()
+        join_event3 = make_event_from_dict(
+            {
+                "room_id": self.room_id,
+                "sender": "@baduser:test.serv",
+                "state_key": "@baduser:test.serv",
+                "event_id": "$join3:test.serv",
+                "depth": 1000,
+                "origin_server_ts": 1,
+                "type": "m.room.power_levels",
+                "origin": "test.servx",
+                "content": {},
+                "auth_events": [],
+                "prev_state": [(most_recent, {})],
+                "prev_events": [(most_recent, {})],
+                "unsigned": {
+                    "malicious garbage": "hackz",
+                    "more warez": "more hackz",
+                    "age": 14,
+                    "invite_room_state": [],
+                },
+            }
+        )
+        self.get_success(
+            federation_event_handler.on_receive_pdu("test.serv", join_event3)
+        )
+
+        event = self.get_success(self.store.get_event("$join3:test.serv"))
+        event_dict = event.get_dict()
+        self.assertTrue("age" in event_dict["unsigned"])
+        self.assertTrue("invite_room_state" not in event_dict["unsigned"])

From d3e786d239835d7c629e791c922222c856eb941b Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Fri, 3 Dec 2021 12:52:32 -0800
Subject: [PATCH 02/19] add function to strip unauthorized fields from the
 unsigned object of event

---
 synapse/handlers/federation_event.py | 37 +++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index 9917613298c6..29f429c82ebf 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -52,7 +52,7 @@
     check_auth_rules_for_event,
     validate_event_for_room_version,
 )
-from synapse.events import EventBase
+from synapse.events import EventBase, make_event_from_dict
 from synapse.events.snapshot import EventContext
 from synapse.federation.federation_client import InvalidResponseError
 from synapse.logging.context import nested_logging_context, run_in_background
@@ -181,6 +181,11 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
             logger.warning("Received event failed sanity checks")
             raise FederationError("ERROR", err.code, err.msg, affected=pdu.event_id)
 
+        # Strip any unauthorized unsigned values if they exist
+        event_dict = pdu.get_dict()
+        if pdu.unsigned and event_dict["unsigned"] != {}:
+            pdu = self.strip_unsigned_values(pdu, event_dict)
+
         # If we are currently in the process of joining this room, then we
         # queue up events for later processing.
         if room_id in self.room_queues:
@@ -1939,3 +1944,33 @@ def _sanity_check_event(self, ev: EventBase) -> None:
                 len(ev.auth_event_ids()),
             )
             raise SynapseError(HTTPStatus.BAD_REQUEST, "Too many auth_events")
+
+    def strip_unsigned_values(self, pdu: EventBase, event_dict: Dict) -> EventBase:
+        """
+        Strip any unsigned values unless specifically allowed, as defined by the whitelist.
+
+        pdu: the event to strip values from
+        event_dict: a dict of values derived from the pdu
+        """
+        unsigned = event_dict.get("unsigned")
+
+        if pdu.type == EventTypes.Member:
+            whitelist = ["knock_room_state", "invite_room_state", "age"]
+        else:
+            whitelist = ["age"]
+
+        filtered_unsigned = {}
+
+        # Unsigned should never be None by the time we are here but mypy doesn't know that
+        assert unsigned is not None
+        for k, v in unsigned.items():
+            if k in whitelist:
+                filtered_unsigned[k] = v
+
+        event_dict["unsigned"] = filtered_unsigned
+
+        stripped_event = make_event_from_dict(
+            event_dict, pdu.room_version, pdu.internal_metadata.get_dict()
+        )
+
+        return stripped_event

From edc3ebb8d8fb27efb006a76157403e825a916367 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Tue, 7 Dec 2021 09:27:13 -0800
Subject: [PATCH 03/19] newsfragment

---
 changelog.d/11528.misc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/11528.misc

diff --git a/changelog.d/11528.misc b/changelog.d/11528.misc
new file mode 100644
index 000000000000..3c546688257d
--- /dev/null
+++ b/changelog.d/11528.misc
@@ -0,0 +1 @@
+Add functionality to strip the unsignedData object of unauthorized fields in events arriving over federation.
\ No newline at end of file

From e60f9a2e64770fca7f6f6364cdbaa8cba0657a0d Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Tue, 7 Dec 2021 09:34:14 -0800
Subject: [PATCH 04/19] update newsfragment number

---
 changelog.d/{11528.misc => 11530.misc} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename changelog.d/{11528.misc => 11530.misc} (100%)

diff --git a/changelog.d/11528.misc b/changelog.d/11530.misc
similarity index 100%
rename from changelog.d/11528.misc
rename to changelog.d/11530.misc

From c865eac939762e20eb62082738837f36fcb96a66 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Tue, 7 Dec 2021 10:09:46 -0800
Subject: [PATCH 05/19] add check to on_send_membership_event

---
 synapse/handlers/federation_event.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index 29f429c82ebf..177a14b9d152 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -330,6 +330,11 @@ async def on_send_membership_event(
 
         assert not event.internal_metadata.outlier
 
+        # Strip any unauthorized unsigned values if they exist
+        event_dict = event.get_dict()
+        if event.unsigned and event_dict["unsigned"] != {}:
+            event = self.strip_unsigned_values(event, event_dict)
+
         # Send this event on behalf of the other server.
         #
         # The remote server isn't a full participant in the room at this point, so

From bd38e539ed0d291a822603456b647f9df4f85312 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Tue, 7 Dec 2021 10:17:10 -0800
Subject: [PATCH 06/19] refactor tests

---
 tests/test_federation.py | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/tests/test_federation.py b/tests/test_federation.py
index 353b0b544792..dda6b68a5238 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -278,7 +278,7 @@ def test_cross_signing_keys_retry(self):
         self.assertTrue(remote_self_signing_key in self_signing_key["keys"].values())
 
 
-class StripEventsTestCase(MessageAcceptTests):
+class StripUnsignedFromEventsTestCase(MessageAcceptTests):
     def setUp(self):
         super().setUp()
 
@@ -287,12 +287,12 @@ def strip_unauthorized_unsigned_values(self):
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
         federation_event_handler = self.homeserver.get_federation_event_handler()
-        join_event1 = make_event_from_dict(
+        event1 = make_event_from_dict(
             {
                 "room_id": self.room_id,
                 "sender": "@baduser:test.serv",
                 "state_key": "@baduser:test.serv",
-                "event_id": "$join1:test.serv",
+                "event_id": "$event1:test.serv",
                 "depth": 1000,
                 "origin_server_ts": 1,
                 "type": "m.room.member",
@@ -304,11 +304,9 @@ def strip_unauthorized_unsigned_values(self):
                 "unsigned": {"malicious garbage": "hackz", "more warez": "more hackz"},
             }
         )
-        self.get_success(
-            federation_event_handler.on_receive_pdu("test.serv", join_event1)
-        )
+        self.get_success(federation_event_handler.on_receive_pdu("test.serv", event1))
 
-        event = self.get_success(self.store.get_event("$join1:test.serv"))
+        event = self.get_success(self.store.get_event("$event1:test.serv"))
         event_dict = event.get_dict()
         self.assertTrue("hackz" not in event_dict["unsigned"])
 
@@ -317,20 +315,20 @@ def strip_event_maintains_allowed_fields(self):
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
         federation_event_handler = self.homeserver.get_federation_event_handler()
-        join_event2 = make_event_from_dict(
+        event2 = make_event_from_dict(
             {
                 "room_id": self.room_id,
                 "sender": "@baduser:test.serv",
                 "state_key": "@baduser:test.serv",
-                "event_id": "$join2:test.serv",
+                "event_id": "$event2:test.serv",
                 "depth": 1000,
                 "origin_server_ts": 1,
                 "type": "m.room.member",
                 "origin": "test.servx",
-                "content": {"membership": "join"},
                 "auth_events": [],
                 "prev_state": [(most_recent, {})],
                 "prev_events": [(most_recent, {})],
+                "content": {"membership": "join"},
                 "unsigned": {
                     "malicious garbage": "hackz",
                     "more warez": "more hackz",
@@ -339,13 +337,12 @@ def strip_event_maintains_allowed_fields(self):
                 },
             }
         )
-        self.get_success(
-            federation_event_handler.on_receive_pdu("test.serv", join_event2)
-        )
+        self.get_success(federation_event_handler.on_send_membership_event("test.serv", event2))
 
-        event = self.get_success(self.store.get_event("$join2:test.serv"))
+        event = self.get_success(self.store.get_event("$event2:test.serv"))
         event_dict = event.get_dict()
         self.assertTrue("age" in event_dict["unsigned"])
+        self.assertTrue("hackz" not in event_dict["unsigned"])
         self.assertTrue("invite_room_state" in event_dict["unsigned"])
 
     def strip_event_removes_fields_based_on_event_type(self):
@@ -353,12 +350,12 @@ def strip_event_removes_fields_based_on_event_type(self):
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
         federation_event_handler = self.homeserver.get_federation_event_handler()
-        join_event3 = make_event_from_dict(
+        event3 = make_event_from_dict(
             {
                 "room_id": self.room_id,
                 "sender": "@baduser:test.serv",
                 "state_key": "@baduser:test.serv",
-                "event_id": "$join3:test.serv",
+                "event_id": "$event3:test.serv",
                 "depth": 1000,
                 "origin_server_ts": 1,
                 "type": "m.room.power_levels",
@@ -375,11 +372,12 @@ def strip_event_removes_fields_based_on_event_type(self):
                 },
             }
         )
-        self.get_success(
-            federation_event_handler.on_receive_pdu("test.serv", join_event3)
-        )
+        self.get_success(federation_event_handler.on_receive_pdu("test.serv", event3))
 
-        event = self.get_success(self.store.get_event("$join3:test.serv"))
+        event = self.get_success(self.store.get_event("$event3:test.serv"))
         event_dict = event.get_dict()
         self.assertTrue("age" in event_dict["unsigned"])
         self.assertTrue("invite_room_state" not in event_dict["unsigned"])
+        self.assertTrue("more warez" not in event_dict["unsigned"])
+
+

From c34088ad9bffe9115905df1db3d7cee8cb0b5e34 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Tue, 7 Dec 2021 10:24:38 -0800
Subject: [PATCH 07/19] fix lint error

---
 tests/test_federation.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test_federation.py b/tests/test_federation.py
index dda6b68a5238..964cf5aa1a7c 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -337,7 +337,9 @@ def strip_event_maintains_allowed_fields(self):
                 },
             }
         )
-        self.get_success(federation_event_handler.on_send_membership_event("test.serv", event2))
+        self.get_success(
+            federation_event_handler.on_send_membership_event("test.serv", event2)
+        )
 
         event = self.get_success(self.store.get_event("$event2:test.serv"))
         event_dict = event.get_dict()
@@ -379,5 +381,3 @@ def strip_event_removes_fields_based_on_event_type(self):
         self.assertTrue("age" in event_dict["unsigned"])
         self.assertTrue("invite_room_state" not in event_dict["unsigned"])
         self.assertTrue("more warez" not in event_dict["unsigned"])
-
-

From 28b2141cd637426cede1972105835f8fee37d805 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 8 Dec 2021 12:41:07 -0800
Subject: [PATCH 08/19] slightly refactor tests and add some comments

---
 tests/test_federation.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/test_federation.py b/tests/test_federation.py
index 964cf5aa1a7c..4cdc0b63821c 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -308,7 +308,8 @@ def strip_unauthorized_unsigned_values(self):
 
         event = self.get_success(self.store.get_event("$event1:test.serv"))
         event_dict = event.get_dict()
-        self.assertTrue("hackz" not in event_dict["unsigned"])
+        # Make sure unauthorized fields are stripped from unsigned
+        self.assertNotIn("hackz", event_dict["unsigned"])
 
     def strip_event_maintains_allowed_fields(self):
         most_recent = self.get_success(
@@ -343,9 +344,10 @@ def strip_event_maintains_allowed_fields(self):
 
         event = self.get_success(self.store.get_event("$event2:test.serv"))
         event_dict = event.get_dict()
-        self.assertTrue("age" in event_dict["unsigned"])
-        self.assertTrue("hackz" not in event_dict["unsigned"])
-        self.assertTrue("invite_room_state" in event_dict["unsigned"])
+        self.assertIn("age", event_dict["unsigned"])
+        self.assertNotIn("hackz", event_dict["unsigned"])
+        # Invite_room_state is allowed in events of type m.room.member
+        self.assertIn("invite_room_state", event_dict["unsigned"])
 
     def strip_event_removes_fields_based_on_event_type(self):
         most_recent = self.get_success(
@@ -378,6 +380,7 @@ def strip_event_removes_fields_based_on_event_type(self):
 
         event = self.get_success(self.store.get_event("$event3:test.serv"))
         event_dict = event.get_dict()
-        self.assertTrue("age" in event_dict["unsigned"])
-        self.assertTrue("invite_room_state" not in event_dict["unsigned"])
-        self.assertTrue("more warez" not in event_dict["unsigned"])
+        self.assertIn("age", event_dict["unsigned"])
+        # Invite_room_state field is only associated with event type m.room.member
+        self.assertNotIn("invite_room_state", event_dict["unsigned"])
+        self.assertNotIn("more warez", event_dict["unsigned"])

From 36696dad323e8fa002ba15a56aeeef7a50b0ad24 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Fri, 10 Dec 2021 12:09:17 -0800
Subject: [PATCH 09/19] slight refactor

---
 synapse/handlers/federation_event.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index 177a14b9d152..59db18b80880 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -28,6 +28,7 @@
     Tuple,
 )
 
+from mypy.build import Any
 from prometheus_client import Counter
 
 from synapse.api.constants import (
@@ -183,7 +184,7 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
 
         # Strip any unauthorized unsigned values if they exist
         event_dict = pdu.get_dict()
-        if pdu.unsigned and event_dict["unsigned"] != {}:
+        if pdu.unsigned and "unsigned" in event_dict and event_dict["unsigned"] != {}:
             pdu = self.strip_unsigned_values(pdu, event_dict)
 
         # If we are currently in the process of joining this room, then we
@@ -332,7 +333,7 @@ async def on_send_membership_event(
 
         # Strip any unauthorized unsigned values if they exist
         event_dict = event.get_dict()
-        if event.unsigned and event_dict["unsigned"] != {}:
+        if event.unsigned and "unsigned" in event_dict and event_dict["unsigned"] != {}:
             event = self.strip_unsigned_values(event, event_dict)
 
         # Send this event on behalf of the other server.
@@ -1950,14 +1951,22 @@ def _sanity_check_event(self, ev: EventBase) -> None:
             )
             raise SynapseError(HTTPStatus.BAD_REQUEST, "Too many auth_events")
 
-    def strip_unsigned_values(self, pdu: EventBase, event_dict: Dict) -> EventBase:
+    def strip_unsigned_values(
+        self, pdu: EventBase, event_dict: Dict[str, Any]
+    ) -> EventBase:
         """
         Strip any unsigned values unless specifically allowed, as defined by the whitelist.
 
         pdu: the event to strip values from
         event_dict: a dict of values derived from the pdu
         """
-        unsigned = event_dict.get("unsigned")
+        unsigned = event_dict["unsigned"]
+
+        if not isinstance(unsigned, dict):
+            event_dict["unsigned"] = {}
+            return make_event_from_dict(
+                event_dict, pdu.room_version, pdu.internal_metadata.get_dict()
+            )
 
         if pdu.type == EventTypes.Member:
             whitelist = ["knock_room_state", "invite_room_state", "age"]
@@ -1966,8 +1975,6 @@ def strip_unsigned_values(self, pdu: EventBase, event_dict: Dict) -> EventBase:
 
         filtered_unsigned = {}
 
-        # Unsigned should never be None by the time we are here but mypy doesn't know that
-        assert unsigned is not None
         for k, v in unsigned.items():
             if k in whitelist:
                 filtered_unsigned[k] = v

From a3e72588d7988a96196aeb0b209e465007c2b066 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Fri, 10 Dec 2021 12:13:18 -0800
Subject: [PATCH 10/19] refactor tests

---
 tests/test_federation.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/tests/test_federation.py b/tests/test_federation.py
index 4cdc0b63821c..e651ed32eee6 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -279,10 +279,7 @@ def test_cross_signing_keys_retry(self):
 
 
 class StripUnsignedFromEventsTestCase(MessageAcceptTests):
-    def setUp(self):
-        super().setUp()
-
-    def strip_unauthorized_unsigned_values(self):
+    def test_strip_unauthorized_unsigned_values(self):
         most_recent = self.get_success(
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
@@ -309,9 +306,9 @@ def strip_unauthorized_unsigned_values(self):
         event = self.get_success(self.store.get_event("$event1:test.serv"))
         event_dict = event.get_dict()
         # Make sure unauthorized fields are stripped from unsigned
-        self.assertNotIn("hackz", event_dict["unsigned"])
+        self.assertNotIn("more warez", event_dict["unsigned"])
 
-    def strip_event_maintains_allowed_fields(self):
+    def test_strip_event_maintains_allowed_fields(self):
         most_recent = self.get_success(
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
@@ -345,11 +342,13 @@ def strip_event_maintains_allowed_fields(self):
         event = self.get_success(self.store.get_event("$event2:test.serv"))
         event_dict = event.get_dict()
         self.assertIn("age", event_dict["unsigned"])
-        self.assertNotIn("hackz", event_dict["unsigned"])
+        self.assertEqual(14, event_dict["unsigned"]["age"])
+        self.assertNotIn("more warez", event_dict["unsigned"])
         # Invite_room_state is allowed in events of type m.room.member
         self.assertIn("invite_room_state", event_dict["unsigned"])
+        self.assertEqual([], event_dict["unsigned"]["invite_room_state"])
 
-    def strip_event_removes_fields_based_on_event_type(self):
+    def test_strip_event_removes_fields_based_on_event_type(self):
         most_recent = self.get_success(
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
@@ -381,6 +380,6 @@ def strip_event_removes_fields_based_on_event_type(self):
         event = self.get_success(self.store.get_event("$event3:test.serv"))
         event_dict = event.get_dict()
         self.assertIn("age", event_dict["unsigned"])
-        # Invite_room_state field is only associated with event type m.room.member
+        # Invite_room_state field is only permitted in event type m.room.member
         self.assertNotIn("invite_room_state", event_dict["unsigned"])
         self.assertNotIn("more warez", event_dict["unsigned"])

From 6475c71016bf636ca4e462a41cf22981cf2b8472 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Tue, 14 Dec 2021 13:09:51 -0800
Subject: [PATCH 11/19] fix import error

---
 synapse/handlers/federation_event.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index 59db18b80880..58e1c98afb93 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -17,6 +17,7 @@
 from http import HTTPStatus
 from typing import (
     TYPE_CHECKING,
+    Any,
     Collection,
     Container,
     Dict,
@@ -28,7 +29,6 @@
     Tuple,
 )
 
-from mypy.build import Any
 from prometheus_client import Counter
 
 from synapse.api.constants import (

From e29625b5dc678a7181ea788c65073b640bd392fd Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Thu, 16 Dec 2021 12:51:55 -0800
Subject: [PATCH 12/19] slight refactor

---
 synapse/handlers/federation_event.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index 58e1c98afb93..bd74e0603cfe 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -184,8 +184,8 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
 
         # Strip any unauthorized unsigned values if they exist
         event_dict = pdu.get_dict()
-        if pdu.unsigned and "unsigned" in event_dict and event_dict["unsigned"] != {}:
-            pdu = self.strip_unsigned_values(pdu, event_dict)
+        if "unsigned" in event_dict and event_dict["unsigned"] != {}:
+            pdu = self._strip_unsigned_values(pdu, event_dict)
 
         # If we are currently in the process of joining this room, then we
         # queue up events for later processing.
@@ -333,8 +333,8 @@ async def on_send_membership_event(
 
         # Strip any unauthorized unsigned values if they exist
         event_dict = event.get_dict()
-        if event.unsigned and "unsigned" in event_dict and event_dict["unsigned"] != {}:
-            event = self.strip_unsigned_values(event, event_dict)
+        if "unsigned" in event_dict and event_dict["unsigned"] != {}:
+            event = self._strip_unsigned_values(event, event_dict)
 
         # Send this event on behalf of the other server.
         #
@@ -1951,14 +1951,15 @@ def _sanity_check_event(self, ev: EventBase) -> None:
             )
             raise SynapseError(HTTPStatus.BAD_REQUEST, "Too many auth_events")
 
-    def strip_unsigned_values(
+    def _strip_unsigned_values(
         self, pdu: EventBase, event_dict: Dict[str, Any]
     ) -> EventBase:
         """
         Strip any unsigned values unless specifically allowed, as defined by the whitelist.
 
         pdu: the event to strip values from
-        event_dict: a dict of values derived from the pdu
+        event_dict: a dict of values derived from the pdu (note that this dict is mutated
+        by this function)
         """
         unsigned = event_dict["unsigned"]
 

From 3451bd422f7cac5c18d6d7b1c05b77f296e0c361 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 22 Dec 2021 13:21:33 -0800
Subject: [PATCH 13/19] remove unsigned filtration code from
 synapse/handlers/federation_event.py

---
 synapse/handlers/federation_event.py | 48 +---------------------------
 1 file changed, 1 insertion(+), 47 deletions(-)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index bd74e0603cfe..091f1863d1c5 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -17,7 +17,6 @@
 from http import HTTPStatus
 from typing import (
     TYPE_CHECKING,
-    Any,
     Collection,
     Container,
     Dict,
@@ -53,7 +52,7 @@
     check_auth_rules_for_event,
     validate_event_for_room_version,
 )
-from synapse.events import EventBase, make_event_from_dict
+from synapse.events import EventBase
 from synapse.events.snapshot import EventContext
 from synapse.federation.federation_client import InvalidResponseError
 from synapse.logging.context import nested_logging_context, run_in_background
@@ -182,11 +181,6 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
             logger.warning("Received event failed sanity checks")
             raise FederationError("ERROR", err.code, err.msg, affected=pdu.event_id)
 
-        # Strip any unauthorized unsigned values if they exist
-        event_dict = pdu.get_dict()
-        if "unsigned" in event_dict and event_dict["unsigned"] != {}:
-            pdu = self._strip_unsigned_values(pdu, event_dict)
-
         # If we are currently in the process of joining this room, then we
         # queue up events for later processing.
         if room_id in self.room_queues:
@@ -331,11 +325,6 @@ async def on_send_membership_event(
 
         assert not event.internal_metadata.outlier
 
-        # Strip any unauthorized unsigned values if they exist
-        event_dict = event.get_dict()
-        if "unsigned" in event_dict and event_dict["unsigned"] != {}:
-            event = self._strip_unsigned_values(event, event_dict)
-
         # Send this event on behalf of the other server.
         #
         # The remote server isn't a full participant in the room at this point, so
@@ -1951,39 +1940,4 @@ def _sanity_check_event(self, ev: EventBase) -> None:
             )
             raise SynapseError(HTTPStatus.BAD_REQUEST, "Too many auth_events")
 
-    def _strip_unsigned_values(
-        self, pdu: EventBase, event_dict: Dict[str, Any]
-    ) -> EventBase:
-        """
-        Strip any unsigned values unless specifically allowed, as defined by the whitelist.
-
-        pdu: the event to strip values from
-        event_dict: a dict of values derived from the pdu (note that this dict is mutated
-        by this function)
-        """
-        unsigned = event_dict["unsigned"]
-
-        if not isinstance(unsigned, dict):
-            event_dict["unsigned"] = {}
-            return make_event_from_dict(
-                event_dict, pdu.room_version, pdu.internal_metadata.get_dict()
-            )
-
-        if pdu.type == EventTypes.Member:
-            whitelist = ["knock_room_state", "invite_room_state", "age"]
-        else:
-            whitelist = ["age"]
-
-        filtered_unsigned = {}
-
-        for k, v in unsigned.items():
-            if k in whitelist:
-                filtered_unsigned[k] = v
-
-        event_dict["unsigned"] = filtered_unsigned
-
-        stripped_event = make_event_from_dict(
-            event_dict, pdu.room_version, pdu.internal_metadata.get_dict()
-        )
 
-        return stripped_event

From b8cfe8cacf95cf64094d69cf61de1b95a6ea64d2 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 22 Dec 2021 13:55:08 -0800
Subject: [PATCH 14/19] lint

---
 synapse/handlers/federation_event.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py
index 091f1863d1c5..9917613298c6 100644
--- a/synapse/handlers/federation_event.py
+++ b/synapse/handlers/federation_event.py
@@ -1939,5 +1939,3 @@ def _sanity_check_event(self, ev: EventBase) -> None:
                 len(ev.auth_event_ids()),
             )
             raise SynapseError(HTTPStatus.BAD_REQUEST, "Too many auth_events")
-
-

From 718e1688d44ec9b602c032279d3b3f82613f81a4 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 22 Dec 2021 13:56:19 -0800
Subject: [PATCH 15/19] move unsigned filtering code to event base

---
 synapse/federation/federation_base.py | 33 +++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py
index f56344a3b94f..28446574f735 100644
--- a/synapse/federation/federation_base.py
+++ b/synapse/federation/federation_base.py
@@ -238,6 +238,10 @@ def event_from_pdu_json(
     # origin, etc etc)
     assert_params_in_dict(pdu_json, ("type", "depth"))
 
+    # Strip any unauthorized values from "unsigned" if they exist
+    if "unsigned" in pdu_json and pdu_json["unsigned"] != {}:
+        pdu_json = _strip_unsigned_values(pdu_json)
+
     depth = pdu_json["depth"]
     if not isinstance(depth, int):
         raise SynapseError(400, "Depth %r not an intger" % (depth,), Codes.BAD_JSON)
@@ -255,3 +259,32 @@ def event_from_pdu_json(
     event.internal_metadata.outlier = outlier
 
     return event
+
+
+def _strip_unsigned_values(pdu_dict: JsonDict) -> JsonDict:
+    """
+    Strip any unsigned values unless specifically allowed, as defined by the whitelist.
+
+    pdu: the json dict to strip values from. Note that the dict is mutated by this
+    function
+    """
+    unsigned = pdu_dict["unsigned"]
+
+    if not isinstance(unsigned, dict):
+        pdu_dict["unsigned"] = {}
+        return pdu_dict
+
+    if pdu_dict["type"] == "m.room.member":
+        whitelist = ["knock_room_state", "invite_room_state", "age"]
+    else:
+        whitelist = ["age"]
+
+    filtered_unsigned = {}
+
+    for k, v in unsigned.items():
+        if k in whitelist:
+            filtered_unsigned[k] = v
+
+    pdu_dict["unsigned"] = filtered_unsigned
+
+    return pdu_dict

From ae06c9ff2c11905b07eb39c2415ae594d6ad544d Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 22 Dec 2021 13:57:35 -0800
Subject: [PATCH 16/19] refactor tests

---
 tests/test_federation.py | 157 +++++++++++++++++----------------------
 1 file changed, 70 insertions(+), 87 deletions(-)

diff --git a/tests/test_federation.py b/tests/test_federation.py
index e651ed32eee6..9bfad8e8806f 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -17,7 +17,9 @@
 from twisted.internet.defer import succeed
 
 from synapse.api.errors import FederationError
+from synapse.api.room_versions import RoomVersions
 from synapse.events import make_event_from_dict
+from synapse.federation.federation_base import event_from_pdu_json
 from synapse.logging.context import LoggingContext
 from synapse.types import UserID, create_requester
 from synapse.util import Clock
@@ -283,103 +285,84 @@ def test_strip_unauthorized_unsigned_values(self):
         most_recent = self.get_success(
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
-        federation_event_handler = self.homeserver.get_federation_event_handler()
-        event1 = make_event_from_dict(
-            {
-                "room_id": self.room_id,
-                "sender": "@baduser:test.serv",
-                "state_key": "@baduser:test.serv",
-                "event_id": "$event1:test.serv",
-                "depth": 1000,
-                "origin_server_ts": 1,
-                "type": "m.room.member",
-                "origin": "test.servx",
-                "content": {"membership": "join"},
-                "auth_events": [],
-                "prev_state": [(most_recent, {})],
-                "prev_events": [(most_recent, {})],
-                "unsigned": {"malicious garbage": "hackz", "more warez": "more hackz"},
-            }
-        )
-        self.get_success(federation_event_handler.on_receive_pdu("test.serv", event1))
-
-        event = self.get_success(self.store.get_event("$event1:test.serv"))
-        event_dict = event.get_dict()
+        event1 = {
+            "room_id": self.room_id,
+            "sender": "@baduser:test.serv",
+            "state_key": "@baduser:test.serv",
+            "event_id": "$event1:test.serv",
+            "depth": 1000,
+            "origin_server_ts": 1,
+            "type": "m.room.member",
+            "origin": "test.servx",
+            "content": {"membership": "join"},
+            "auth_events": [],
+            "prev_state": [(most_recent, {})],
+            "prev_events": [(most_recent, {})],
+            "unsigned": {"malicious garbage": "hackz", "more warez": "more hackz"},
+        }
+        filtered_event = event_from_pdu_json(event1, RoomVersions.V1)
         # Make sure unauthorized fields are stripped from unsigned
-        self.assertNotIn("more warez", event_dict["unsigned"])
+        self.assertNotIn("more warez", filtered_event.unsigned)
 
     def test_strip_event_maintains_allowed_fields(self):
         most_recent = self.get_success(
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
-        federation_event_handler = self.homeserver.get_federation_event_handler()
-        event2 = make_event_from_dict(
-            {
-                "room_id": self.room_id,
-                "sender": "@baduser:test.serv",
-                "state_key": "@baduser:test.serv",
-                "event_id": "$event2:test.serv",
-                "depth": 1000,
-                "origin_server_ts": 1,
-                "type": "m.room.member",
-                "origin": "test.servx",
-                "auth_events": [],
-                "prev_state": [(most_recent, {})],
-                "prev_events": [(most_recent, {})],
-                "content": {"membership": "join"},
-                "unsigned": {
-                    "malicious garbage": "hackz",
-                    "more warez": "more hackz",
-                    "age": 14,
-                    "invite_room_state": [],
-                },
-            }
-        )
-        self.get_success(
-            federation_event_handler.on_send_membership_event("test.serv", event2)
-        )
-
-        event = self.get_success(self.store.get_event("$event2:test.serv"))
-        event_dict = event.get_dict()
-        self.assertIn("age", event_dict["unsigned"])
-        self.assertEqual(14, event_dict["unsigned"]["age"])
-        self.assertNotIn("more warez", event_dict["unsigned"])
+        event2 = {
+            "room_id": self.room_id,
+            "sender": "@baduser:test.serv",
+            "state_key": "@baduser:test.serv",
+            "event_id": "$event2:test.serv",
+            "depth": 1000,
+            "origin_server_ts": 1,
+            "type": "m.room.member",
+            "origin": "test.servx",
+            "auth_events": [],
+            "prev_state": [(most_recent, {})],
+            "prev_events": [(most_recent, {})],
+            "content": {"membership": "join"},
+            "unsigned": {
+                "malicious garbage": "hackz",
+                "more warez": "more hackz",
+                "age": 14,
+                "invite_room_state": [],
+            },
+        }
+
+        filtered_event2 = event_from_pdu_json(event2, RoomVersions.V1)
+        self.assertIn("age", filtered_event2.unsigned)
+        self.assertEqual(14, filtered_event2.unsigned["age"])
+        self.assertNotIn("more warez", filtered_event2.unsigned)
         # Invite_room_state is allowed in events of type m.room.member
-        self.assertIn("invite_room_state", event_dict["unsigned"])
-        self.assertEqual([], event_dict["unsigned"]["invite_room_state"])
+        self.assertIn("invite_room_state", filtered_event2.unsigned)
+        self.assertEqual([], filtered_event2.unsigned["invite_room_state"])
 
     def test_strip_event_removes_fields_based_on_event_type(self):
         most_recent = self.get_success(
             self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
         )[0]
-        federation_event_handler = self.homeserver.get_federation_event_handler()
-        event3 = make_event_from_dict(
-            {
-                "room_id": self.room_id,
-                "sender": "@baduser:test.serv",
-                "state_key": "@baduser:test.serv",
-                "event_id": "$event3:test.serv",
-                "depth": 1000,
-                "origin_server_ts": 1,
-                "type": "m.room.power_levels",
-                "origin": "test.servx",
-                "content": {},
-                "auth_events": [],
-                "prev_state": [(most_recent, {})],
-                "prev_events": [(most_recent, {})],
-                "unsigned": {
-                    "malicious garbage": "hackz",
-                    "more warez": "more hackz",
-                    "age": 14,
-                    "invite_room_state": [],
-                },
-            }
-        )
-        self.get_success(federation_event_handler.on_receive_pdu("test.serv", event3))
-
-        event = self.get_success(self.store.get_event("$event3:test.serv"))
-        event_dict = event.get_dict()
-        self.assertIn("age", event_dict["unsigned"])
+        event3 = {
+            "room_id": self.room_id,
+            "sender": "@baduser:test.serv",
+            "state_key": "@baduser:test.serv",
+            "event_id": "$event3:test.serv",
+            "depth": 1000,
+            "origin_server_ts": 1,
+            "type": "m.room.power_levels",
+            "origin": "test.servx",
+            "content": {},
+            "auth_events": [],
+            "prev_state": [(most_recent, {})],
+            "prev_events": [(most_recent, {})],
+            "unsigned": {
+                "malicious garbage": "hackz",
+                "more warez": "more hackz",
+                "age": 14,
+                "invite_room_state": [],
+            },
+        }
+        filtered_event3 = event_from_pdu_json(event3, RoomVersions.V1)
+        self.assertIn("age", filtered_event3.unsigned)
         # Invite_room_state field is only permitted in event type m.room.member
-        self.assertNotIn("invite_room_state", event_dict["unsigned"])
-        self.assertNotIn("more warez", event_dict["unsigned"])
+        self.assertNotIn("invite_room_state", filtered_event3.unsigned)
+        self.assertNotIn("more warez", filtered_event3.unsigned)

From f0e9b97a2365628a4d8a9d41ce1b729c1a16c6b5 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 22 Dec 2021 14:02:47 -0800
Subject: [PATCH 17/19] update newsfragment

---
 changelog.d/11530.bugfix | 2 ++
 changelog.d/11530.misc   | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 100644 changelog.d/11530.bugfix
 delete mode 100644 changelog.d/11530.misc

diff --git a/changelog.d/11530.bugfix b/changelog.d/11530.bugfix
new file mode 100644
index 000000000000..7ea9ba4e4988
--- /dev/null
+++ b/changelog.d/11530.bugfix
@@ -0,0 +1,2 @@
+Fix a long-standing issue which could cause Synapse to incorrectly accept data in the unsigned field of events
+received over federation.
\ No newline at end of file
diff --git a/changelog.d/11530.misc b/changelog.d/11530.misc
deleted file mode 100644
index 3c546688257d..000000000000
--- a/changelog.d/11530.misc
+++ /dev/null
@@ -1 +0,0 @@
-Add functionality to strip the unsignedData object of unauthorized fields in events arriving over federation.
\ No newline at end of file

From 091799102689153752c9d95d47216363a371858a Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Wed, 5 Jan 2022 13:45:39 -0800
Subject: [PATCH 18/19] requested changes

---
 synapse/federation/federation_base.py | 11 +++--------
 tests/test_federation.py              | 20 +-------------------
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py
index 28446574f735..c11cb21f503e 100644
--- a/synapse/federation/federation_base.py
+++ b/synapse/federation/federation_base.py
@@ -239,8 +239,8 @@ def event_from_pdu_json(
     assert_params_in_dict(pdu_json, ("type", "depth"))
 
     # Strip any unauthorized values from "unsigned" if they exist
-    if "unsigned" in pdu_json and pdu_json["unsigned"] != {}:
-        pdu_json = _strip_unsigned_values(pdu_json)
+    if "unsigned" in pdu_json:
+        _strip_unsigned_values(pdu_json)
 
     depth = pdu_json["depth"]
     if not isinstance(depth, int):
@@ -279,12 +279,7 @@ def _strip_unsigned_values(pdu_dict: JsonDict) -> JsonDict:
     else:
         whitelist = ["age"]
 
-    filtered_unsigned = {}
-
-    for k, v in unsigned.items():
-        if k in whitelist:
-            filtered_unsigned[k] = v
-
+    filtered_unsigned = {k: v for k, v in unsigned.items() if k in whitelist}
     pdu_dict["unsigned"] = filtered_unsigned
 
     return pdu_dict
diff --git a/tests/test_federation.py b/tests/test_federation.py
index 9bfad8e8806f..2b9804aba005 100644
--- a/tests/test_federation.py
+++ b/tests/test_federation.py
@@ -280,13 +280,9 @@ def test_cross_signing_keys_retry(self):
         self.assertTrue(remote_self_signing_key in self_signing_key["keys"].values())
 
 
-class StripUnsignedFromEventsTestCase(MessageAcceptTests):
+class StripUnsignedFromEventsTestCase(unittest.TestCase):
     def test_strip_unauthorized_unsigned_values(self):
-        most_recent = self.get_success(
-            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
-        )[0]
         event1 = {
-            "room_id": self.room_id,
             "sender": "@baduser:test.serv",
             "state_key": "@baduser:test.serv",
             "event_id": "$event1:test.serv",
@@ -296,8 +292,6 @@ def test_strip_unauthorized_unsigned_values(self):
             "origin": "test.servx",
             "content": {"membership": "join"},
             "auth_events": [],
-            "prev_state": [(most_recent, {})],
-            "prev_events": [(most_recent, {})],
             "unsigned": {"malicious garbage": "hackz", "more warez": "more hackz"},
         }
         filtered_event = event_from_pdu_json(event1, RoomVersions.V1)
@@ -305,11 +299,7 @@ def test_strip_unauthorized_unsigned_values(self):
         self.assertNotIn("more warez", filtered_event.unsigned)
 
     def test_strip_event_maintains_allowed_fields(self):
-        most_recent = self.get_success(
-            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
-        )[0]
         event2 = {
-            "room_id": self.room_id,
             "sender": "@baduser:test.serv",
             "state_key": "@baduser:test.serv",
             "event_id": "$event2:test.serv",
@@ -318,8 +308,6 @@ def test_strip_event_maintains_allowed_fields(self):
             "type": "m.room.member",
             "origin": "test.servx",
             "auth_events": [],
-            "prev_state": [(most_recent, {})],
-            "prev_events": [(most_recent, {})],
             "content": {"membership": "join"},
             "unsigned": {
                 "malicious garbage": "hackz",
@@ -338,11 +326,7 @@ def test_strip_event_maintains_allowed_fields(self):
         self.assertEqual([], filtered_event2.unsigned["invite_room_state"])
 
     def test_strip_event_removes_fields_based_on_event_type(self):
-        most_recent = self.get_success(
-            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
-        )[0]
         event3 = {
-            "room_id": self.room_id,
             "sender": "@baduser:test.serv",
             "state_key": "@baduser:test.serv",
             "event_id": "$event3:test.serv",
@@ -352,8 +336,6 @@ def test_strip_event_removes_fields_based_on_event_type(self):
             "origin": "test.servx",
             "content": {},
             "auth_events": [],
-            "prev_state": [(most_recent, {})],
-            "prev_events": [(most_recent, {})],
             "unsigned": {
                 "malicious garbage": "hackz",
                 "more warez": "more hackz",

From 835a282164f1d9e4959d2b627ee90aa3aec18122 Mon Sep 17 00:00:00 2001
From: "H. Shay" <hillerys@element.io>
Date: Thu, 6 Jan 2022 08:27:27 -0800
Subject: [PATCH 19/19] remove unused retun values

---
 synapse/federation/federation_base.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py
index 7af44a85b145..896168c05c0a 100644
--- a/synapse/federation/federation_base.py
+++ b/synapse/federation/federation_base.py
@@ -251,7 +251,7 @@ def event_from_pdu_json(pdu_json: JsonDict, room_version: RoomVersion) -> EventB
     return event
 
 
-def _strip_unsigned_values(pdu_dict: JsonDict) -> JsonDict:
+def _strip_unsigned_values(pdu_dict: JsonDict) -> None:
     """
     Strip any unsigned values unless specifically allowed, as defined by the whitelist.
 
@@ -262,7 +262,6 @@ def _strip_unsigned_values(pdu_dict: JsonDict) -> JsonDict:
 
     if not isinstance(unsigned, dict):
         pdu_dict["unsigned"] = {}
-        return pdu_dict
 
     if pdu_dict["type"] == "m.room.member":
         whitelist = ["knock_room_state", "invite_room_state", "age"]
@@ -271,5 +270,3 @@ def _strip_unsigned_values(pdu_dict: JsonDict) -> JsonDict:
 
     filtered_unsigned = {k: v for k, v in unsigned.items() if k in whitelist}
     pdu_dict["unsigned"] = filtered_unsigned
-
-    return pdu_dict