diff --git a/src/api/client/membership/join.rs b/src/api/client/membership/join.rs index 953211a1..2d16a70e 100644 --- a/src/api/client/membership/join.rs +++ b/src/api/client/membership/join.rs @@ -48,7 +48,7 @@ use service::{ }, }; -use super::banned_room_check; +use super::{banned_room_check, validate_remote_member_event_stub}; use crate::Ruma; /// # `POST /_matrix/client/r0/rooms/{roomId}/join` @@ -837,6 +837,13 @@ async fn join_room_by_id_helper_local( err!(BadServerResponse("Invalid make_join event json received from server: {e:?}")) })?; + validate_remote_member_event_stub( + &MembershipState::Join, + sender_user, + room_id, + &join_event_stub, + )?; + let join_authorized_via_users_server = join_event_stub .get("content") .map(|s| { diff --git a/src/api/client/membership/knock.rs b/src/api/client/membership/knock.rs index 39db96d0..0ecbe2f5 100644 --- a/src/api/client/membership/knock.rs +++ b/src/api/client/membership/knock.rs @@ -38,7 +38,7 @@ use service::{ }, }; -use super::{banned_room_check, join::join_room_by_id_helper}; +use super::{banned_room_check, join::join_room_by_id_helper, validate_remote_member_event_stub}; use crate::Ruma; /// # `POST /_matrix/client/*/knock/{roomIdOrAlias}` @@ -408,6 +408,13 @@ async fn knock_room_helper_local( err!(BadServerResponse("Invalid make_knock event json received from server: {e:?}")) })?; + validate_remote_member_event_stub( + &MembershipState::Knock, + sender_user, + room_id, + &knock_event_stub, + )?; + knock_event_stub.insert( "origin".to_owned(), CanonicalJsonValue::String(services.globals.server_name().as_str().to_owned()), diff --git a/src/api/client/membership/leave.rs b/src/api/client/membership/leave.rs index 4ef4a2b0..90fbbb86 100644 --- a/src/api/client/membership/leave.rs +++ b/src/api/client/membership/leave.rs @@ -21,6 +21,7 @@ use ruma::{ }; use service::Services; +use super::validate_remote_member_event_stub; use crate::Ruma; /// # `POST /_matrix/client/v3/rooms/{roomId}/leave` @@ -324,6 +325,13 @@ pub async fn remote_leave_room( ))) })?; + validate_remote_member_event_stub( + &MembershipState::Leave, + user_id, + room_id, + &leave_event_stub, + )?; + // TODO: Is origin needed? leave_event_stub.insert( "origin".to_owned(), diff --git a/src/api/client/membership/mod.rs b/src/api/client/membership/mod.rs index 5e742a69..9123bac8 100644 --- a/src/api/client/membership/mod.rs +++ b/src/api/client/membership/mod.rs @@ -13,7 +13,14 @@ use std::net::IpAddr; use axum::extract::State; use conduwuit::{Err, Result, warn}; use futures::{FutureExt, StreamExt}; -use ruma::{OwnedRoomId, RoomId, ServerName, UserId, api::client::membership::joined_rooms}; +use ruma::{ + CanonicalJsonObject, OwnedRoomId, RoomId, ServerName, UserId, + api::client::membership::joined_rooms, + events::{ + StaticEventContent, + room::member::{MembershipState, RoomMemberEventContent}, + }, +}; use service::Services; pub(crate) use self::{ @@ -153,3 +160,80 @@ pub(crate) async fn banned_room_check( Ok(()) } + +/// Validates that an event returned from a remote server by `/make_*` +/// actually is a membership event with the expected fields. +/// +/// Without checking this, the remote server could use the remote membership +/// mechanism to trick our server into signing arbitrary malicious events. +pub(crate) fn validate_remote_member_event_stub( + membership: &MembershipState, + user_id: &UserId, + room_id: &RoomId, + event_stub: &CanonicalJsonObject, +) -> Result<()> { + let Some(event_type) = event_stub.get("type") else { + return Err!(BadServerResponse( + "Remote server returned member event with missing type field" + )); + }; + if event_type != &RoomMemberEventContent::TYPE { + return Err!(BadServerResponse( + "Remote server returned member event with invalid event type" + )); + } + + let Some(sender) = event_stub.get("sender") else { + return Err!(BadServerResponse( + "Remote server returned member event with missing sender field" + )); + }; + if sender != &user_id.as_str() { + return Err!(BadServerResponse( + "Remote server returned member event with incorrect sender" + )); + } + + let Some(state_key) = event_stub.get("state_key") else { + return Err!(BadServerResponse( + "Remote server returned member event with missing state_key field" + )); + }; + if state_key != &user_id.as_str() { + return Err!(BadServerResponse( + "Remote server returned member event with incorrect state_key" + )); + } + + let Some(event_room_id) = event_stub.get("room_id") else { + return Err!(BadServerResponse( + "Remote server returned member event with missing room_id field" + )); + }; + if event_room_id != &room_id.as_str() { + return Err!(BadServerResponse( + "Remote server returned member event with incorrect room_id" + )); + } + + let Some(content) = event_stub + .get("content") + .and_then(|content| content.as_object()) + else { + return Err!(BadServerResponse( + "Remote server returned member event with missing content field" + )); + }; + let Some(event_membership) = content.get("membership") else { + return Err!(BadServerResponse( + "Remote server returned member event with missing membership field" + )); + }; + if event_membership != &membership.as_str() { + return Err!(BadServerResponse( + "Remote server returned member event with incorrect room_id" + )); + } + + Ok(()) +}