Do checks on aliases for incoming m.room.aliases events (#5128)

Follow-up to #5124

Also added a bunch of checks to make sure everything (both the stuff added on #5124 and this PR) works as intended.

5 files changed, 55 insertions, 8 deletions

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 0860b75905..8547a63535 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -20,6 +20,9 @@
 # the "depth" field on events is limited to 2**63 - 1
 MAX_DEPTH = 2**63 - 1
 
+# the maximum length for a room alias is 255 characters
+MAX_ALIAS_LENGTH = 255
+
 
 class Membership(object):
 
diff --git a/synapse/events/snapshot.py b/synapse/events/snapshot.py
index 368b5f6ae4..fa09c132a0 100644
--- a/synapse/events/snapshot.py
+++ b/synapse/events/snapshot.py
@@ -187,7 +187,9 @@ class EventContext(object):
 
         Returns:
             Deferred[dict[(str, str), str]|None]: Returns None if state_group
-            is None, which happens when the associated event is an outlier.
+                is None, which happens when the associated event is an outlier.
+                Maps a (type, state_key) to the event ID of the state event matching
+                this tuple.
         """
 
         if not self._fetching_state_deferred:
@@ -205,7 +207,9 @@ class EventContext(object):
 
         Returns:
             Deferred[dict[(str, str), str]|None]: Returns None if state_group
-            is None, which happens when the associated event is an outlier.
+                is None, which happens when the associated event is an outlier.
+                Maps a (type, state_key) to the event ID of the state event matching
+                this tuple.
         """
 
         if not self._fetching_state_deferred:
diff --git a/synapse/events/validator.py b/synapse/events/validator.py
index 514273c792..711af512b2 100644
--- a/synapse/events/validator.py
+++ b/synapse/events/validator.py
@@ -15,8 +15,8 @@
 
 from six import string_types
 
-from synapse.api.constants import EventTypes, Membership
-from synapse.api.errors import SynapseError
+from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership
+from synapse.api.errors import Codes, SynapseError
 from synapse.api.room_versions import EventFormatVersions
 from synapse.types import EventID, RoomID, UserID
 
@@ -56,6 +56,17 @@ class EventValidator(object):
             if not isinstance(getattr(event, s), string_types):
                 raise SynapseError(400, "'%s' not a string type" % (s,))
 
+        if event.type == EventTypes.Aliases:
+            if "aliases" in event.content:
+                for alias in event.content["aliases"]:
+                    if len(alias) > MAX_ALIAS_LENGTH:
+                        raise SynapseError(
+                            400,
+                            ("Can't create aliases longer than"
+                             " %d characters" % (MAX_ALIAS_LENGTH,)),
+                            Codes.INVALID_PARAM,
+                        )
+
     def validate_builder(self, event):
         """Validates that the builder/event has roughly the right format. Only
         checks values that we expect a proto event to have, rather than all the
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 50c587aa61..a12f9508d8 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -19,7 +19,7 @@ import string
 
 from twisted.internet import defer
 
-from synapse.api.constants import EventTypes
+from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes
 from synapse.api.errors import (
     AuthError,
     CodeMessageException,
@@ -36,7 +36,6 @@ logger = logging.getLogger(__name__)
 
 
 class DirectoryHandler(BaseHandler):
-    MAX_ALIAS_LENGTH = 255
 
     def __init__(self, hs):
         super(DirectoryHandler, self).__init__(hs)
@@ -105,10 +104,10 @@ class DirectoryHandler(BaseHandler):
 
         user_id = requester.user.to_string()
 
-        if len(room_alias.to_string()) > self.MAX_ALIAS_LENGTH:
+        if len(room_alias.to_string()) > MAX_ALIAS_LENGTH:
             raise SynapseError(
                 400,
-                "Can't create aliases longer than %s characters" % self.MAX_ALIAS_LENGTH,
+                "Can't create aliases longer than %s characters" % MAX_ALIAS_LENGTH,
                 Codes.INVALID_PARAM,
             )
 
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 224d34ef3a..e5afeadf68 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -228,6 +228,7 @@ class EventCreationHandler(object):
         self.ratelimiter = hs.get_ratelimiter()
         self.notifier = hs.get_notifier()
         self.config = hs.config
+        self.require_membership_for_aliases = hs.config.require_membership_for_aliases
 
         self.send_event_to_master = ReplicationSendEventRestServlet.make_client(hs)
 
@@ -336,6 +337,35 @@ class EventCreationHandler(object):
             prev_events_and_hashes=prev_events_and_hashes,
         )
 
+        # In an ideal world we wouldn't need the second part of this condition. However,
+        # this behaviour isn't spec'd yet, meaning we should be able to deactivate this
+        # behaviour. Another reason is that this code is also evaluated each time a new
+        # m.room.aliases event is created, which includes hitting a /directory route.
+        # Therefore not including this condition here would render the similar one in
+        # synapse.handlers.directory pointless.
+        if builder.type == EventTypes.Aliases and self.require_membership_for_aliases:
+            # Ideally we'd do the membership check in event_auth.check(), which
+            # describes a spec'd algorithm for authenticating events received over
+            # federation as well as those created locally. As of room v3, aliases events
+            # can be created by users that are not in the room, therefore we have to
+            # tolerate them in event_auth.check().
+            prev_state_ids = yield context.get_prev_state_ids(self.store)
+            prev_event_id = prev_state_ids.get((EventTypes.Member, event.sender))
+            prev_event = yield self.store.get_event(prev_event_id, allow_none=True)
+            if not prev_event or prev_event.membership != Membership.JOIN:
+                logger.warning(
+                    ("Attempt to send `m.room.aliases` in room %s by user %s but"
+                     " membership is %s"),
+                    event.room_id,
+                    event.sender,
+                    prev_event.membership if prev_event else None,
+                )
+
+                raise AuthError(
+                    403,
+                    "You must be in the room to create an alias for it",
+                )
+
         self.validator.validate_new(event)
 
         defer.returnValue((event, context))