From 051221c0ab05830f7f93daa0060ad4c573918059 Mon Sep 17 00:00:00 2001
From: Olivia Lee <olivia@computer.surgery>
Date: Mon, 20 Jan 2025 12:11:41 -0800
Subject: [PATCH] validate schema of new canonical alias events sent by clients

Previously, we would only attempt to validate the aliases in the event
content if we were able to parse the event, and would silently allow it
otherwise.
---
 book/changelog.md              |  3 ++
 src/api/client_server/state.rs | 56 +++++++++++++++++++---------------
 2 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/book/changelog.md b/book/changelog.md
index 6dfe833b..0ee07aa1 100644
--- a/book/changelog.md
+++ b/book/changelog.md
@@ -225,6 +225,9 @@ This will be the first release of Grapevine since it was forked from Conduit
     instead of 400 M_FORBIDDEN when trying to set a canonical alias that does
     not exist.
     ([!158](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/158))
+26. Validate schema of new `m.room.canonical_alias` event sent by clients,
+    rather than silently allowing any contents if the event can't be parsed.
+    ([!158](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/158))
 
 ### Added
 
diff --git a/src/api/client_server/state.rs b/src/api/client_server/state.rs
index a9a6c0fc..54621603 100644
--- a/src/api/client_server/state.rs
+++ b/src/api/client_server/state.rs
@@ -210,33 +210,39 @@ async fn send_state_event_for_key_helper(
 ) -> Result<Arc<EventId>> {
     let sender_user = sender;
 
-    // TODO: Review this check, error if event is unparsable, use event type,
-    // allow alias if it previously existed
-    if let Ok(canonical_alias) = serde_json::from_str::<
-        RoomCanonicalAliasEventContent,
-    >(json.json().get())
-    {
-        let mut aliases = canonical_alias.alt_aliases.clone();
+    // TODO: allow alias if it previously existed
+    if event_type == &StateEventType::RoomCanonicalAlias {
+        if let Ok(canonical_alias) = serde_json::from_str::<
+            RoomCanonicalAliasEventContent,
+        >(json.json().get())
+        {
+            let mut aliases = canonical_alias.alt_aliases.clone();
 
-        if let Some(alias) = canonical_alias.alias {
-            aliases.push(alias);
-        }
-
-        for alias in aliases {
-            if alias.server_name() != services().globals.server_name()
-                || services()
-                    .rooms
-                    .alias
-                    .resolve_local_alias(&alias)?
-                    .filter(|room| room == room_id)
-                    .is_none()
-            {
-                return Err(Error::BadRequest(
-                    ErrorKind::BadAlias,
-                    "You are only allowed to send canonical_alias events when \
-                     it's aliases already exists",
-                ));
+            if let Some(alias) = canonical_alias.alias {
+                aliases.push(alias);
             }
+
+            for alias in aliases {
+                if alias.server_name() != services().globals.server_name()
+                    || services()
+                        .rooms
+                        .alias
+                        .resolve_local_alias(&alias)?
+                        .filter(|room| room == room_id)
+                        .is_none()
+                {
+                    return Err(Error::BadRequest(
+                        ErrorKind::BadAlias,
+                        "You are only allowed to send canonical_alias events \
+                         when it's aliases already exists",
+                    ));
+                }
+            }
+        } else {
+            return Err(Error::BadRequest(
+                ErrorKind::InvalidParam,
+                "m.room.canonical_alias event did not match expected schema",
+            ));
         }
     }