From da99b0706e683a2d347768efe5b50676abdf7b44 Mon Sep 17 00:00:00 2001 From: Matthias Ahouansou Date: Wed, 12 Jun 2024 10:36:41 -0700 Subject: [PATCH] fix(edus): ensure sender server is the same as the user in the content Original patch by Matthias. Benjamin modified the logic to include logging when an event was rejected, for consistency with the existing check on device key updates. Co-authored-by: Benjamin Lee --- src/api/server_server.rs | 111 +++++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 40 deletions(-) diff --git a/src/api/server_server.rs b/src/api/server_server.rs index 81ec6c58..cd0386b8 100644 --- a/src/api/server_server.rs +++ b/src/api/server_server.rs @@ -812,6 +812,15 @@ pub(crate) async fn send_transaction_message_route( Edu::Receipt(receipt) => { for (room_id, room_updates) in receipt.receipts { for (user_id, user_updates) in room_updates.read { + if user_id.server_name() != sender_servername { + warn!( + %user_id, + %sender_servername, + "Got receipt EDU from incorrect homeserver, \ + ignoring", + ); + continue; + } if let Some((event_id, _)) = user_updates .event_ids .iter() @@ -859,6 +868,14 @@ pub(crate) async fn send_transaction_message_route( } } Edu::Typing(typing) => { + if typing.user_id.server_name() != sender_servername { + warn!( + user_id = %typing.user_id, + %sender_servername, + "Got typing EDU from incorrect homeserver, ignoring", + ); + continue; + } if services() .rooms .state_cache @@ -889,6 +906,15 @@ pub(crate) async fn send_transaction_message_route( user_id, .. }) => { + if user_id.server_name() != sender_servername { + warn!( + %user_id, + %sender_servername, + "Got device list update EDU from incorrect homeserver, \ + ignoring", + ); + continue; + } services().users.mark_device_key_update(&user_id)?; } Edu::DirectToDevice(DirectDeviceContent { @@ -897,22 +923,27 @@ pub(crate) async fn send_transaction_message_route( message_id, messages, }) => { + if sender.server_name() != sender_servername { + warn!( + user_id = %sender, + %sender_servername, + "Got direct-to-device EDU from incorrect homeserver, \ + ignoring", + ); + continue; + } // Check if this is a new transaction id if services() .transaction_ids .existing_txnid(&sender, None, &message_id)? - .is_some() + .is_none() { - continue; - } - - for (target_user_id, map) in &messages { - for (target_device_id_maybe, event) in map { - match target_device_id_maybe { - DeviceIdOrAllDevices::DeviceId( - target_device_id, - ) => { - services().users.add_to_device_event( + for (target_user_id, map) in &messages { + for (target_device_id_maybe, event) in map { + match target_device_id_maybe { + DeviceIdOrAllDevices::DeviceId( + target_device_id, + ) => services().users.add_to_device_event( &sender, target_user_id, target_device_id, @@ -927,41 +958,41 @@ pub(crate) async fn send_transaction_message_route( "Event is invalid", ) })?, - )?; - } + )?, - DeviceIdOrAllDevices::AllDevices => { - for target_device_id in services() - .users - .all_device_ids(target_user_id) - { - services().users.add_to_device_event( - &sender, - target_user_id, - &target_device_id?, - &ev_type.to_string(), - event.deserialize_as().map_err( - |_| { - Error::BadRequest( - ErrorKind::InvalidParam, - "Event is invalid", - ) - }, - )?, - )?; + DeviceIdOrAllDevices::AllDevices => { + for target_device_id in services() + .users + .all_device_ids(target_user_id) + { + services().users.add_to_device_event( + &sender, + target_user_id, + &target_device_id?, + &ev_type.to_string(), + event.deserialize_as().map_err( + |_| { + Error::BadRequest( + ErrorKind::InvalidParam, + "Event is invalid", + ) + }, + )?, + )?; + } } } } } - } - // Save transaction id with empty data - services().transaction_ids.add_txnid( - &sender, - None, - &message_id, - &[], - )?; + // Save transaction id with empty data + services().transaction_ids.add_txnid( + &sender, + None, + &message_id, + &[], + )?; + } } Edu::SigningKeyUpdate(SigningKeyUpdateContent { user_id,