From 4d487afe264c4cea74c0fb4298840e391ecd92dd Mon Sep 17 00:00:00 2001 From: timedout Date: Sat, 22 Aug 2026 20:47:06 +0100 Subject: [PATCH] fix: SEC30 Reviewed-By: Ginger Reviewed-By: Jade Reviewed-By: Jacob Taylor --- src/core/matrix/state_res/event_auth.rs | 462 +++++++++------------- src/core/matrix/state_res/power_levels.rs | 24 -- 2 files changed, 183 insertions(+), 303 deletions(-) diff --git a/src/core/matrix/state_res/event_auth.rs b/src/core/matrix/state_res/event_auth.rs index 8abc4a25e..f085e2a41 100644 --- a/src/core/matrix/state_res/event_auth.rs +++ b/src/core/matrix/state_res/event_auth.rs @@ -7,26 +7,22 @@ use futures::{ use ruma::{ Int, OwnedUserId, RoomVersionId, UserId, events::room::{ - create::RoomCreateEventContent, join_rules::{JoinRule, RoomJoinRulesEventContent}, member::{MembershipState, ThirdPartyInvite}, - power_levels::RoomPowerLevelsEventContent, + power_levels::{RoomPowerLevelsEventContent, UserPowerLevel}, }, int, room_version_rules::{RoomIdFormatVersion, RoomVersionRules}, serde::Raw, }; -use serde::{ - Deserialize, - de::{Error as _, IgnoredAny}, -}; +use serde::Deserialize; use serde_json::{from_str as from_json_str, value::RawValue as RawJsonValue}; use super::{ Error, Event, Result, StateEventType, StateKey, TimelineEventType, power_levels::{ - deserialize_power_levels, deserialize_power_levels_content_fields, - deserialize_power_levels_content_invite, deserialize_power_levels_content_redact, + deserialize_power_levels, deserialize_power_levels_content_invite, + deserialize_power_levels_content_redact, }, }; use crate::{debug, error, trace, warn}; @@ -46,7 +42,7 @@ struct RoomMemberContentFields { #[derive(Deserialize)] struct RoomCreateContentFields { room_version: Option>, - creator: Option>, + creator: Option>, additional_creators: Option>>, #[serde(rename = "m.federate", default = "ruma::serde::default_true")] federate: bool, @@ -137,6 +133,31 @@ pub fn auth_types_for_event( Ok(auth_types) } +fn get_user_power_level( + room_version_rules: &RoomVersionRules, + creator: &UserId, + additional_creators: &[Raw], + power_levels: Option<&RoomPowerLevelsEventContent>, + user_id: &UserId, +) -> UserPowerLevel { + if room_version_rules + .authorization + .explicitly_privilege_room_creators + && (user_id == creator + || additional_creators + .iter() + .any(|c| c.deserialize().ok() == Some(user_id.to_owned()))) + { + return UserPowerLevel::Infinite; + } + + if let Some(pls) = power_levels { + UserPowerLevel::Int(pls.users.get(user_id).copied().unwrap_or(pls.users_default)) + } else { + UserPowerLevel::Int(if user_id == creator { int!(100) } else { int!(0) }) + } +} + /// Authenticate the incoming `event`. /// /// The steps of authentication are: @@ -478,69 +499,43 @@ where return Ok(false); } - // If type is m.room.third_party_invite - let mut sender_power_level = match &power_levels_event { - | Some(pl) => { - let content = - deserialize_power_levels_content_fields(pl.content().get(), room_version)?; - match content.get_user_power(sender) { - | Some(level) => *level, - | _ => content.users_default, - } - }, - | _ => { - // If no power level event found the creator gets 100 everyone else gets 0 - let is_creator = if room_version.authorization.use_room_create_sender { - room_create_event.sender() == sender - } else { - #[allow(deprecated)] - from_json_str::(room_create_event.content().get()) - .is_ok_and(|create| create.creator.unwrap() == *sender) - }; - - if is_creator { int!(100) } else { int!(0) } - }, + let creator = if room_version.authorization.use_room_create_sender { + room_create_event.sender().to_owned() + } else { + room_create_content + .creator + .clone() + .ok_or_else(|| { + Error::InvalidPdu("Missing creator field in m.room.create event".to_owned()) + })? + .deserialize()? }; - if room_version - .authorization - .explicitly_privilege_room_creators - { - // If the user sent the create event, or is listed in additional_creators, just - // give them Int::MAX - if sender == room_create_event.sender() - || room_create_content - .additional_creators - .as_ref() - .is_some_and(|creators| { - creators - .iter() - .any(|c| c.deserialize().is_ok_and(|c| c == *sender)) - }) { - trace!("privileging room creator or additional creator"); - // This user is the room creator or an additional creator, give them max power - // level - sender_power_level = Int::MAX; - } - } + let power_levels: Option = power_levels_event + .as_ref() + .map(|pl| from_json_str(pl.content().get())) + .transpose()?; + let effective_power_levels = power_levels + .clone() + .unwrap_or_else(|| RoomPowerLevelsEventContent::new(&room_version.authorization)); + let sender_power_level = get_user_power_level( + room_version, + &creator, + room_create_content + .additional_creators + .as_deref() + .unwrap_or_default(), + power_levels.as_ref(), + sender, + ); // Allow if and only if sender's current power level is greater than // or equal to the invite level if *incoming_event.event_type() == TimelineEventType::RoomThirdPartyInvite { - let invite_level = match &power_levels_event { - | Some(power_levels) => - deserialize_power_levels_content_invite( - power_levels.content().get(), - room_version, - )? - .invite, - | None => int!(0), - }; - - if sender_power_level < invite_level { + if sender_power_level < effective_power_levels.invite { warn!( %sender, - has=%sender_power_level, - required=%invite_level, + has=?sender_power_level, + required=%effective_power_levels.invite, "sender cannot send invites in this room" ); return Ok(false); @@ -615,7 +610,7 @@ where if !check_redaction(room_version, incoming_event, sender_power_level, redact_level)? { warn!( %sender, - %sender_power_level, + ?sender_power_level, %redact_level, "redaction event was not allowed" ); @@ -627,34 +622,6 @@ where Ok(true) } -fn is_creator( - v: &RoomVersionRules, - c: &BTreeSet, - ce: &EV, - user_id: &UserId, - have_pls: bool, -) -> bool -where - EV: Event + Send + Sync, -{ - if v.authorization.explicitly_privilege_room_creators { - c.contains(user_id) - } else if v.authorization.use_room_create_sender && !have_pls { - ce.sender() == user_id - } else if !have_pls { - #[allow(deprecated)] - let creator = from_json_str::(ce.content().get()) - .unwrap() - .creator - .ok_or_else(|| serde_json::Error::missing_field("creator")) - .unwrap(); - - creator == user_id - } else { - false - } -} - // TODO deserializing the member, power, join_rules event contents is done in // conduit just before this is called. Could they be passed in? /// Does the user who sent this member event have required power levels to do @@ -691,6 +658,17 @@ where third_party_invite: Option>, } let create_content = from_json_str::(create_room.content().get())?; + let creator = if room_version.authorization.use_room_create_sender { + create_room.sender().to_owned() + } else { + create_content + .creator + .clone() + .ok_or_else(|| { + Error::InvalidPdu("Missing creator field in m.room.create event".to_owned()) + })? + .deserialize()? + }; let content = current_event.content(); let target_membership = from_json_str::(content.get())?.membership; @@ -708,44 +686,33 @@ where | None => MembershipState::Leave, }; - let power_levels: RoomPowerLevelsEventContent = match &power_levels_event { - | Some(ev) => from_json_str(ev.content().get())?, - | None => RoomPowerLevelsEventContent::new(&room_version.authorization), - }; + let power_levels: Option = power_levels_event + .map(|ev| from_json_str(ev.content().get())) + .transpose()?; + let effective_power_levels: RoomPowerLevelsEventContent = power_levels + .clone() + .unwrap_or_else(|| RoomPowerLevelsEventContent::new(&room_version.authorization)); - let mut sender_power = power_levels - .users - .get(sender) - .or_else(|| sender_is_joined.then_some(&power_levels.users_default)); - - let mut target_power = power_levels.users.get(target_user).or_else(|| { - (target_membership == MembershipState::Join).then_some(&power_levels.users_default) - }); - - let mut creators = BTreeSet::new(); - creators.insert(create_room.sender().to_owned()); - if room_version - .authorization - .explicitly_privilege_room_creators - { - // Explicitly privilege room creators - // If the sender sent the create event, or in additional_creators, give them - // Int::MAX. Same case for target. - if let Some(additional_creators) = &create_content.additional_creators { - for c in additional_creators { - if let Ok(c) = c.deserialize() { - creators.insert(c); - } - } - } - if creators.contains(sender) { - sender_power = Some(&Int::MAX); - } - if creators.contains(target_user) { - target_power = Some(&Int::MAX); - } - } - trace!(?creators, "creators for room"); + let sender_power = get_user_power_level( + room_version, + &creator, + create_content + .additional_creators + .as_deref() + .unwrap_or_default(), + power_levels.as_ref(), + sender, + ); + let target_power = get_user_power_level( + room_version, + &creator, + create_content + .additional_creators + .as_deref() + .unwrap_or_default(), + power_levels.as_ref(), + target_user, + ); let join_rules = if let Some(jr) = &join_rules_event { from_json_str::(jr.content().get())?.join_rule @@ -765,54 +732,29 @@ where let invite = deserialize_power_levels_content_invite(pl.content().get(), room_version)?.invite; - let content = - deserialize_power_levels_content_fields(pl.content().get(), room_version)?; - let user_pl = match content.get_user_power(user_for_join_auth) { - | Some(level) => *level, - | _ => content.users_default, - }; + let user_pl = get_user_power_level( + room_version, + &creator, + create_content + .additional_creators + .as_deref() + .unwrap_or_default(), + power_levels.as_ref(), + user_for_join_auth, + ); (user_pl, invite) } else { - (int!(0), int!(0)) + (UserPowerLevel::Int(int!(0)), int!(0)) }; let user_joined = user_for_join_auth_membership == &MembershipState::Join; - let okay_power = is_creator( - room_version, - &creators, - create_room, - user_for_join_auth, - power_levels_event.as_ref().is_some(), - ) || auth_user_pl >= invite_level; - trace!( - %auth_user_pl, - %auth_user_pl, - %invite_level, - %user_joined, - %okay_power, - passing=%(user_joined && okay_power), - "user for join auth is valid check details" - ); + let okay_power = auth_user_pl >= invite_level; user_joined && okay_power } else { // No auth user was given trace!("No auth user given for join auth"); false }; - let sender_creator = is_creator( - room_version, - &creators, - create_room, - sender, - power_levels_event.as_ref().is_some(), - ); - let target_creator = is_creator( - room_version, - &creators, - create_room, - target_user, - power_levels_event.as_ref().is_some(), - ); Ok(match target_membership { | MembershipState::Join => { @@ -828,16 +770,7 @@ where let no_more_prev_events = prev_events.next().is_none(); if prev_event_is_create_event && no_more_prev_events { - trace!( - %sender, - target_user = %target_user, - ?sender_creator, - ?target_creator, - "checking if sender is a room creator for initial membership event" - ); - let is_creator = sender_creator && target_creator; - - if is_creator { + if sender == creator { debug!("sender is room creator, allowing join"); return Ok(true); } @@ -1001,15 +934,12 @@ where ); false } else { - let allow = sender_creator - || sender_power - .as_ref() - .is_some_and(|&p| p >= &power_levels.invite); - if !allow { + let allow = sender_power < effective_power_levels.invite; + if allow { warn!( %sender, has=?sender_power, - required=?power_levels.invite, + required=?effective_power_levels.invite, "sender does not have enough power to produce invites", ); } @@ -1020,7 +950,7 @@ where ?target_user_membership_event_id, ?target_user_current_membership, sender_pl=?sender_power, - required_pl=?power_levels.invite, + required_pl=?effective_power_levels.invite, "allowing invite" ); allow @@ -1028,47 +958,9 @@ where } }, | MembershipState::Leave => { - let can_unban = if target_user_current_membership == MembershipState::Ban { - sender_creator - || sender_power - .as_ref() - .is_some_and(|&p| p >= &power_levels.ban) - } else { - true - }; - let can_kick = if !matches!( - target_user_current_membership, - MembershipState::Ban | MembershipState::Leave - ) { - if sender_creator { - // sender is a creator - true - } else if sender_power - .as_ref() - .is_none_or(|&p| p < &power_levels.kick) - { - // sender lacks kick power level - false - } else if let Some(sp) = sender_power { - if let Some(tp) = target_power { - // sender must have more power than target - sp > tp - } else { - // target has default power level - true - } - } else { - // sender has default power level - false - } - } else { - true - }; if sender == target_user { - // self-leave - // let allow = target_user_current_membership == MembershipState::Join - // || target_user_current_membership == MembershipState::Invite - // || target_user_current_membership == MembershipState::Knock; + // If the sender matches state_key, allow if that user's current membership + // state is invite, join, or knock. let allow = matches!( target_user_current_membership, MembershipState::Join | MembershipState::Invite | MembershipState::Knock @@ -1084,48 +976,52 @@ where trace!(sender=%sender, "allowing leave"); allow } else if !sender_is_joined { + // If the sender's current membership state is not join, reject. warn!( %sender, ?sender_membership_event_id, "sender cannot kick another user as they are not joined to the room", ); false - } else if !(can_unban && can_kick) { - // If the target is banned, only a room creator or someone with ban power - // level can unban them - warn!( - %sender, - ?target_user_membership_event_id, - ?power_levels_event_id, - "sender lacks the power level required to unban users", - ); - false - } else if !can_kick { - warn!( - %sender, - %target_user, - ?target_user_membership_event_id, - ?target_user_current_membership, - ?power_levels_event_id, - "sender does not have enough power to kick the target", - ); - false } else { - trace!( - %sender, - %target_user, - ?target_user_membership_event_id, - ?target_user_current_membership, - sender_pl=?sender_power, - target_pl=?target_power, - required_pl=?power_levels.kick, - "allowing kick/unban", - ); - true + // If the target user's current membership state is ban, and the sender's power + // level is less than the ban level, reject. If the sender's power level is + // greater than or equal to the kick level, and the target user's power level is + // less than the sender's power level, allow. + let can_unban = target_user_current_membership == MembershipState::Ban + && sender_power < effective_power_levels.ban; + let can_kick = + sender_power >= effective_power_levels.kick && target_power < sender_power; + if !can_unban { + warn!( + %sender, + ?target_user_membership_event_id, + ?power_levels_event_id, + "sender lacks the power level required to unban users", + ); + false + } else if can_kick { + trace!(%sender, "allowing kick or unban"); + true + } else { + warn!( + %sender, + %target_user, + ?target_user_membership_event_id, + ?target_user_current_membership, + ?power_levels_event_id, + sender_pl=?sender_power, + target_pl=?target_power, + required_pl=?effective_power_levels.kick, + "sender does not have enough power to kick the target", + ); + false + } } }, | MembershipState::Ban => if !sender_is_joined { + // If the sender's current membership state is not join, reject. warn!( %sender, ?sender_membership_event_id, @@ -1133,11 +1029,11 @@ where ); false } else { - let allow = sender_creator - || (sender_power - .as_ref() - .is_some_and(|&p| p >= &power_levels.ban) - && target_power < sender_power); + // If the sender's power level is greater than or equal to the ban level, and + // the target user's power level is less than the sender's power level, allow. + // Otherwise, reject. + let allow = + sender_power >= effective_power_levels.ban && target_power < sender_power; if !allow { warn!( %sender, @@ -1213,13 +1109,19 @@ where /// /// Does the event have the correct userId as its state_key if it's not the "" /// state_key. -fn can_send_event(event: &impl Event, ple: Option<&impl Event>, user_level: Int) -> bool { - // TODO(hydra): This function does not care about creators! +fn can_send_event( + event: &impl Event, + ple: Option<&impl Event>, + user_level: UserPowerLevel, +) -> bool { + if user_level == UserPowerLevel::Infinite { + return true; + } let event_type_power_level = get_send_level(event.event_type(), event.state_key(), ple); debug!( required_level = i64::from(event_type_power_level), - user_level = i64::from(user_level), + ?user_level, state_key = ?event.state_key(), power_level_event_id = ?ple.map(|e| e.event_id().as_str()), "permissions factors", @@ -1233,7 +1135,7 @@ fn can_send_event(event: &impl Event, ple: Option<&impl Event>, user_level: Int) && event.state_key() != Some(event.sender().as_str()) { warn!( - %user_level, + ?user_level, required=%event_type_power_level, state_key=?event.state_key(), sender=%event.sender(), @@ -1251,7 +1153,7 @@ fn check_power_levels( room_version: &RoomVersionRules, power_event: &impl Event, previous_power_event: Option<&impl Event>, - user_level: Int, + user_level: UserPowerLevel, creators: &BTreeSet, ) -> Option { match power_event.state_key() { @@ -1327,12 +1229,14 @@ fn check_power_levels( } // If the current value is equal to the sender's current power level, reject - if user != power_event.sender() && old_level == Some(&user_level) { + if user != power_event.sender() + && old_level.map(|i| UserPowerLevel::Int(*i)) == Some(user_level) + { warn!( ?old_level, ?new_level, ?user, - %user_level, + ?user_level, sender=%power_event.sender(), "cannot alter the power level of a user with the same power level as sender's own" ); @@ -1341,14 +1245,14 @@ fn check_power_levels( // If the current value is higher than the sender's current power level, reject // If the new value is higher than the sender's current power level, reject - let old_level_too_big = old_level > Some(&user_level); - let new_level_too_big = new_level > Some(&user_level); + let old_level_too_big = old_level.map(|i| UserPowerLevel::Int(*i)) > Some(user_level); + let new_level_too_big = new_level.map(|i| UserPowerLevel::Int(*i)) > Some(user_level); if old_level_too_big { warn!( ?old_level, ?new_level, ?user, - %user_level, + ?user_level, sender=%power_event.sender(), "cannot alter the power level of a user with a higher power level than sender's own" ); @@ -1359,7 +1263,7 @@ fn check_power_levels( ?old_level, ?new_level, ?user, - %user_level, + ?user_level, sender=%power_event.sender(), "cannot set the power level of a user to a level higher than sender's own" ); @@ -1377,14 +1281,14 @@ fn check_power_levels( // If the current value is higher than the sender's current power level, reject // If the new value is higher than the sender's current power level, reject - let old_level_too_big = old_level > Some(&user_level); - let new_level_too_big = new_level > Some(&user_level); + let old_level_too_big = old_level.map(|i| UserPowerLevel::Int(*i)) > Some(user_level); + let new_level_too_big = new_level.map(|i| UserPowerLevel::Int(*i)) > Some(user_level); if old_level_too_big { warn!( ?old_level, ?new_level, ?ev_type, - %user_level, + ?user_level, sender=%power_event.sender(), "cannot alter the power level of an event with a higher power level than sender's own" ); @@ -1395,7 +1299,7 @@ fn check_power_levels( ?old_level, ?new_level, ?ev_type, - %user_level, + ?user_level, sender=%power_event.sender(), "cannot set the power level of an event to a level higher than sender's own" ); @@ -1416,7 +1320,7 @@ fn check_power_levels( warn!( ?old_level, ?new_level, - %user_level, + ?user_level, sender=%power_event.sender(), "cannot alter the power level of notifications greater than sender's own" ); @@ -1446,7 +1350,7 @@ fn check_power_levels( warn!( ?old_lvl, ?new_lvl, - %user_level, + ?user_level, sender=%power_event.sender(), action=%lvl_name, "cannot alter the power level of action greater than sender's own", @@ -1475,7 +1379,7 @@ fn get_deserialize_levels( fn check_redaction( _room_version: &RoomVersionRules, redaction_event: &impl Event, - user_level: Int, + user_level: UserPowerLevel, redact_level: Int, ) -> Result { if user_level >= redact_level { diff --git a/src/core/matrix/state_res/power_levels.rs b/src/core/matrix/state_res/power_levels.rs index 6b76b4c84..d6b3c4108 100644 --- a/src/core/matrix/state_res/power_levels.rs +++ b/src/core/matrix/state_res/power_levels.rs @@ -180,30 +180,6 @@ impl From for PowerLevelsContentFields { } } -#[inline] -pub(crate) fn deserialize_power_levels_content_fields( - content: &str, - room_version: &RoomVersionRules, -) -> Result { - if room_version.authorization.integer_power_levels { - deserialize_integer_power_levels_content_fields(content) - } else { - deserialize_legacy_power_levels_content_fields(content) - } -} - -fn deserialize_integer_power_levels_content_fields( - content: &str, -) -> Result { - from_json_str::(content).map(Into::into) -} - -fn deserialize_legacy_power_levels_content_fields( - content: &str, -) -> Result { - from_json_str(content) -} - #[derive(Deserialize)] pub(crate) struct PowerLevelsContentInvite { #[serde(default, deserialize_with = "deserialize_v1_powerlevel")]