From 0f20d37ef19bb6395c1dddb9c43e22b1d5a3fcbf Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Tue, 30 Aug 2022 18:29:42 +0100 Subject: [PATCH] Fix error in `is_mine_id` when encountering a malformed ID Fixes #13040. Signed-off-by: Sean Quah --- changelog.d/13746.bugfix | 1 + synapse/server.py | 12 +++++++++++- tests/test_types.py | 20 +++++++++++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 changelog.d/13746.bugfix diff --git a/changelog.d/13746.bugfix b/changelog.d/13746.bugfix new file mode 100644 index 000000000000..b692af8fd506 --- /dev/null +++ b/changelog.d/13746.bugfix @@ -0,0 +1 @@ +Fix a long standing bug where Synapse would fail to handle malformed user IDs or room aliases gracefully in certain cases. diff --git a/synapse/server.py b/synapse/server.py index 5a99c0b3449f..df3a1cb40558 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -341,7 +341,17 @@ def is_mine(self, domain_specific_string: DomainSpecificString) -> bool: return domain_specific_string.domain == self.hostname def is_mine_id(self, string: str) -> bool: - return string.split(":", 1)[1] == self.hostname + """Determines whether a user ID or room alias originates from this homeserver. + + Returns: + `True` if the hostname part of the user ID or room alias matches this + homeserver. + `False` otherwise, or if the user ID or room alias is malformed. + """ + localpart_hostname = string.split(":", 1) + if len(localpart_hostname) < 2: + return False + return localpart_hostname[1] == self.hostname @cache_in_self def get_clock(self) -> Clock: diff --git a/tests/test_types.py b/tests/test_types.py index d8d82a517ea9..55117974c14d 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -13,11 +13,29 @@ # limitations under the License. from synapse.api.errors import SynapseError -from synapse.types import RoomAlias, UserID, map_username_to_mxid_localpart +from synapse.types import RoomAlias, UserID, get_domain_from_id, get_localpart_from_id, map_username_to_mxid_localpart from tests import unittest +class IsMineIDTests(unittest.HomeserverTestCase): + def test_is_mine_id(self) -> None: + self.assertTrue(self.hs.is_mine_id("@user:test")) + self.assertTrue(self.hs.is_mine_id("#room:test")) + self.assertTrue(self.hs.is_mine_id("invalid:test")) + + self.assertFalse(self.hs.is_mine_id("@user:test\0")) + self.assertFalse(self.hs.is_mine_id("@user")) + + def test_two_colons(self) -> None: + """Test handling of IDs containing more than one colon.""" + # The domain starts after the first colon. + # These functions must interpret things consistently. + self.assertFalse(self.hs.is_mine_id("@user:test:test")) + self.assertEqual("user", get_localpart_from_id("@user:test:test")) + self.assertEqual("test:test", get_domain_from_id("@user:test:test")) + + class UserIDTestCase(unittest.HomeserverTestCase): def test_parse(self): user = UserID.from_string("@1234abcd:test")