From 1631c0afa4ff8d1bdb01d015c666c06f81732cab Mon Sep 17 00:00:00 2001 From: timedout Date: Sat, 13 Dec 2025 21:36:20 +0000 Subject: [PATCH] fix: Perform additional validation on events --- src/core/matrix/state_res/event_auth.rs | 5 -- .../event_handler/handle_incoming_pdu.rs | 9 ++- .../rooms/event_handler/handle_outlier_pdu.rs | 11 +++- src/service/rooms/timeline/create.rs | 57 ++++++++++++++----- src/service/rooms/timeline/mod.rs | 2 +- 5 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/core/matrix/state_res/event_auth.rs b/src/core/matrix/state_res/event_auth.rs index f99d4a5e..7fec1f6a 100644 --- a/src/core/matrix/state_res/event_auth.rs +++ b/src/core/matrix/state_res/event_auth.rs @@ -177,11 +177,6 @@ where // [synapse] do_sig_check check the event has valid signatures for member events - // TODO do_size_check is false when called by `iterative_auth_check` - // do_size_check is also mostly accomplished by ruma with the exception of - // checking event_type, state_key, and json are below a certain size (255 and - // 65_536 respectively) - let sender = incoming_event.sender(); // Implementation of https://spec.matrix.org/latest/rooms/v1/#authorization-rules diff --git a/src/service/rooms/event_handler/handle_incoming_pdu.rs b/src/service/rooms/event_handler/handle_incoming_pdu.rs index 5299e8d4..a5d070e9 100644 --- a/src/service/rooms/event_handler/handle_incoming_pdu.rs +++ b/src/service/rooms/event_handler/handle_incoming_pdu.rs @@ -14,7 +14,7 @@ use futures::{ use ruma::{CanonicalJsonValue, EventId, RoomId, ServerName, UserId, events::StateEventType}; use tracing::debug; -use crate::rooms::timeline::RawPduId; +use crate::rooms::timeline::{RawPduId, pdu_fits}; /// When receiving an event one needs to: /// 0. Check the server is in the room @@ -62,6 +62,13 @@ pub async fn handle_incoming_pdu<'a>( if let Ok(pdu_id) = self.services.timeline.get_pdu_id(event_id).await { return Ok(Some(pdu_id)); } + if !pdu_fits(&mut value.clone()) { + warn!( + "dropping incoming PDU {event_id} in room {room_id} from {origin} because it \ + exceeds 65535 bytes or is otherwise too large." + ); + return Err!(Request(TooLarge("PDU is too large"))); + } // 1.1 Check the server is in the room let meta_exists = self.services.metadata.exists(room_id).map(Ok); diff --git a/src/service/rooms/event_handler/handle_outlier_pdu.rs b/src/service/rooms/event_handler/handle_outlier_pdu.rs index 2f4a10b8..5f8bbe76 100644 --- a/src/service/rooms/event_handler/handle_outlier_pdu.rs +++ b/src/service/rooms/event_handler/handle_outlier_pdu.rs @@ -1,7 +1,8 @@ use std::collections::{BTreeMap, HashMap, hash_map}; use conduwuit::{ - Err, Event, PduEvent, Result, debug, debug_info, debug_warn, err, implement, state_res, trace, + Err, Event, PduEvent, Result, debug, debug_info, debug_warn, err, implement, state_res, + trace, warn, }; use futures::future::ready; use ruma::{ @@ -10,6 +11,7 @@ use ruma::{ }; use super::{check_room_id, get_room_version_id, to_room_version}; +use crate::rooms::timeline::pdu_fits; #[implement(super::Service)] #[allow(clippy::too_many_arguments)] @@ -25,6 +27,13 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( where Pdu: Event + Send + Sync, { + if !pdu_fits(&mut value.clone()) { + warn!( + "dropping incoming PDU {event_id} in room {room_id} from {origin} because it \ + exceeds 65535 bytes or is otherwise too large." + ); + return Err!(Request(TooLarge("PDU is too large"))); + } // 1. Remove unsigned field value.remove("unsigned"); diff --git a/src/service/rooms/timeline/create.rs b/src/service/rooms/timeline/create.rs index fc275c1a..793f0309 100644 --- a/src/service/rooms/timeline/create.rs +++ b/src/service/rooms/timeline/create.rs @@ -23,6 +23,40 @@ use serde_json::value::{RawValue, to_raw_value}; use super::RoomMutexGuard; +pub fn pdu_fits(owned_obj: &mut CanonicalJsonObject) -> bool { + // room IDs, event IDs, senders, types, and state keys must all be <= 255 bytes + if let Some(CanonicalJsonValue::String(room_id)) = owned_obj.get("room_id") { + if room_id.len() > 255 { + return false; + } + } + if let Some(CanonicalJsonValue::String(event_id)) = owned_obj.get("event_id") { + if event_id.len() > 255 { + return false; + } + } + if let Some(CanonicalJsonValue::String(sender)) = owned_obj.get("sender") { + if sender.len() > 255 { + return false; + } + } + if let Some(CanonicalJsonValue::String(kind)) = owned_obj.get("type") { + if kind.len() > 255 { + return false; + } + } + if let Some(CanonicalJsonValue::String(state_key)) = owned_obj.get("state_key") { + if state_key.len() > 255 { + return false; + } + } + // Now check the full PDU size + match serde_json::to_string(owned_obj) { + | Ok(s) => s.len() <= 65535, + | Err(_) => false, + } +} + #[implement(super::Service)] pub async fn create_hash_and_sign_event( &self, @@ -148,19 +182,6 @@ pub async fn create_hash_and_sign_event( } } - // if event_type != TimelineEventType::RoomCreate && prev_events.is_empty() { - // return Err!(Request(Unknown("Event incorrectly had zero prev_events."))); - // } - // if state_key.is_none() && depth.lt(&uint!(2)) { - // // The first two events in a room are always m.room.create and - // m.room.member, // so any other events with that same depth are illegal. - // warn!( - // "Had unsafe depth {depth} when creating non-state event in {}. Cowardly - // aborting", room_id.expect("room_id is Some here").as_str() - // ); - // return Err!(Request(Unknown("Unsafe depth for non-state event."))); - // } - let mut pdu = PduEvent { event_id: ruma::event_id!("$thiswillbefilledinlater").into(), room_id: room_id.map(ToOwned::to_owned), @@ -269,8 +290,16 @@ pub async fn create_hash_and_sign_event( } // Generate event id pdu.event_id = gen_event_id(&pdu_json, &room_version_id)?; - // Check with the policy server pdu_json.insert("event_id".into(), CanonicalJsonValue::String(pdu.event_id.clone().into())); + // Verify that the *full* PDU isn't over 64KiB. + // Ruma only validates that it's under 64KiB before signing and hashing. + // Has to be cloned to prevent mutating pdu_json itself :( + if !pdu_fits(&mut pdu_json.clone()) { + // feckin huge PDU mate + return Err!(Request(TooLarge("Message/PDU is too long (exceeds 65535 bytes)"))); + } + + // Check with the policy server if room_id.is_some() { trace!( "Checking event in room {} with policy server", diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs index a19eab9b..252e7bb5 100644 --- a/src/service/rooms/timeline/mod.rs +++ b/src/service/rooms/timeline/mod.rs @@ -26,7 +26,7 @@ use ruma::{ use serde::Deserialize; use self::data::Data; -pub use self::data::PdusIterItem; +pub use self::{create::pdu_fits, data::PdusIterItem}; use crate::{ Dep, account_data, admin, appservice, globals, pusher, rooms, sending, server_keys, users, };