diff --git a/book/changelog.md b/book/changelog.md index c01b7c48..96c850b5 100644 --- a/book/changelog.md +++ b/book/changelog.md @@ -63,6 +63,9 @@ This will be the first release of Grapevine since it was forked from Conduit malicious remote server to trick the local server into signing arbitrary events via remote leave. ([!206](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/206)) +10. Fix vulnerability that allows a malicious server to trick a Grapevine server + into signing forged invite/join events from local users. + ([!207](https://gitlab.computer.surgery/matrix/grapevine/-/merge_requests/207)) ### Removed diff --git a/src/api/server_server.rs b/src/api/server_server.rs index 9696d6ea..bdf93442 100644 --- a/src/api/server_server.rs +++ b/src/api/server_server.rs @@ -59,7 +59,7 @@ use ruma::{ uint, user_id, CanonicalJsonObject, CanonicalJsonValue, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedRoomId, OwnedServerName, OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId, - ServerName, Signatures, + ServerName, Signatures, UserId, }; use serde_json::value::{to_raw_value, RawValue as RawJsonValue}; use tokio::sync::RwLock; @@ -1651,7 +1651,12 @@ async fn create_join_event( )); }; - validate_remote_member_event(&MembershipState::Join, &value)?; + validate_remote_member_event( + &MembershipState::Join, + sender_servername, + room_id, + &value, + )?; let origin: OwnedServerName = serde_json::from_value( serde_json::to_value(value.get("origin").ok_or(Error::BadRequest( @@ -1776,6 +1781,8 @@ pub(crate) async fn create_join_event_v2_route( /// endpoints to trick our server into signing arbitrary malicious events. fn validate_remote_member_event( membership: &MembershipState, + origin: &ServerName, + room_id: &RoomId, event: &CanonicalJsonObject, ) -> Result<()> { let Some(event_type) = event.get("type") else { @@ -1791,6 +1798,62 @@ fn validate_remote_member_event( )); } + let Some(sender) = event.get("sender").and_then(|sender| sender.as_str()) + else { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event sender property is missing or not a string", + )); + }; + let Ok(sender) = UserId::parse(sender) else { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event sender is not a valid user ID", + )); + }; + if sender.server_name() != origin { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event sender is not on the server that sent the request", + )); + } + + if membership == &MembershipState::Invite { + let Some(state_key) = + event.get("state_key").and_then(|state_key| state_key.as_str()) + else { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event state_key property is missing or not a string", + )); + }; + let Ok(state_key) = UserId::parse(state_key) else { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event state_key is not a valid user ID", + )); + }; + if state_key.server_name() != services().globals.server_name() { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Invite event recipient is not a user on this server", + )); + } + } + + let Some(event_room_id) = event.get("room_id") else { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event missing room_id property", + )); + }; + if event_room_id != &room_id.as_str() { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event room_id does not match room ID in endpoint url", + )); + } + let Some(content) = event.get("content").and_then(|content| content.as_object()) else { @@ -1856,7 +1919,12 @@ pub(crate) async fn create_invite_route( ) })?; - validate_remote_member_event(&MembershipState::Invite, &signed_event)?; + validate_remote_member_event( + &MembershipState::Invite, + sender_servername, + &body.room_id, + &signed_event, + )?; ruma::signatures::hash_and_sign_event( services().globals.server_name().as_str(),