fix: Enforce limits when joining rooms

src/api/client/membership/join.rs

@@ -44,6 +44,7 @@ use service::{
 	rooms::{
 		state::RoomMutexGuard,
 		state_compressor::{CompressedState, HashSetCompressStateEvent},
+		timeline::pdu_fits,
 	},
 };
 
@@ -573,6 +574,13 @@ async fn join_room_by_id_helper_remote(
 					return state;
 				},
 			};
+			if !pdu_fits(&mut value.clone()) {
+				warn!(
+					"dropping incoming PDU {event_id} in room {room_id} from room join because \
+					 it exceeds 65535 bytes or is otherwise too large."
+				);
+				return state;
+			}
 			services.rooms.outlier.add_pdu_outlier(&event_id, &value);
 			if let Some(state_key) = &pdu.state_key {
 				let shortstatekey = services

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

src/service/globals/mod.rs

@@ -31,12 +31,13 @@ impl crate::Service for Service {
 
 		let turn_secret = config.turn_secret_file.as_ref().map_or_else(
 			|| config.turn_secret.clone(),
-			|path| {
+			|path| match std::fs::read_to_string(path) {
-				std::fs::read_to_string(path).unwrap_or_else(|e| {
+				| Ok(secret) => secret.trim().to_owned(),
+				| Err(e) => {
 					error!("Failed to read the TURN secret file: {e}");
 
 					config.turn_secret.clone()
-				})
+				},
 			},
 		);
 
@@ -49,7 +50,7 @@ impl crate::Service for Service {
 					return config.registration_token.clone();
 				};
 
-				Some(token)
+				Some(token.trim().to_owned())
 			},
 		);
 

src/service/migrations.rs

@@ -590,6 +590,10 @@ async fn fix_readreceiptid_readreceipt_duplicates(services: &Services) -> Result
 
 const FIXED_CORRUPT_MSC4133_FIELDS_MARKER: &[u8] = b"fix_corrupt_msc4133_fields";
 async fn fix_corrupt_msc4133_fields(services: &Services) -> Result {
+	// Due to an old bug, some conduwuit databases have `us.cloke.msc4175.tz` user
+	// profile fields with raw strings instead of quoted JSON ones.
+	// This migration fixes that.
+
 	use serde_json::{Value, from_slice};
 	type KeyVal<'a> = ((OwnedUserId, String), &'a [u8]);
 
@@ -606,24 +610,28 @@ async fn fix_corrupt_msc4133_fields(services: &Services) -> Result {
 			async |(mut total, mut fixed),
 			       ((user, key), value): KeyVal<'_>|
 			       -> Result<(usize, usize)> {
-				if let Err(error) = from_slice::<Value>(value) {
+				match from_slice::<Value>(value) {
-					// Due to an old bug, some conduwuit databases have `us.cloke.msc4175.tz` user
+					// corrupted timezone field
-					// profile fields with raw strings instead of quoted JSON ones.
+					| Err(_) if key == "us.cloke.msc4175.tz" => {
-					// This migration fixes that.
+						let new_value = Value::String(String::from_utf8(value.to_vec())?);
-					let new_value = if key == "us.cloke.msc4175.tz" {
+						useridprofilekey_value.put((user, key), Json(new_value));
-						Value::String(String::from_utf8(value.to_vec())?)
+						fixed = fixed.saturating_add(1);
-					} else {
+					},
-						return Err!(
+					// corrupted value for some other key
-							"failed to deserialize msc4133 key {} of user {}: {}",
+					| Err(error) => {
-							key,
+						warn!(
-							user,
+							"deleting MSC4133 key {} for user {} due to deserialization \
-							error
+							 failure: {}",
+							key, user, error
 						);
-					};
+						useridprofilekey_value.del((user, key));
-
+					},
-					useridprofilekey_value.put((user, key), Json(new_value));
+					// other key with no issues
-					fixed = fixed.saturating_add(1);
+					| Ok(_) => {
+						// do nothing
+					},
 				}
+
 				total = total.saturating_add(1);
 
 				Ok((total, fixed))

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);

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");
 

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",

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,
 };