From 00d6aeddb6c43660326277a675fadb08a1617c10 Mon Sep 17 00:00:00 2001 From: Matthias Ahouansou Date: Tue, 23 Apr 2024 21:42:01 +0100 Subject: [PATCH] refactor(redactions): move checks inside conduit ruma was already accidentally performing these checks for us, but this shouldn't be the case --- src/service/rooms/event_handler/mod.rs | 15 +++++++- src/service/rooms/state_accessor/mod.rs | 51 +++++++++++++++++++++++++ src/service/rooms/timeline/mod.rs | 35 ++++++++++++++++- 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/src/service/rooms/event_handler/mod.rs b/src/service/rooms/event_handler/mod.rs index 1547d406..ada289fb 100644 --- a/src/service/rooms/event_handler/mod.rs +++ b/src/service/rooms/event_handler/mod.rs @@ -24,7 +24,7 @@ use ruma::{ }, events::{ room::{create::RoomCreateEventContent, server_acl::RoomServerAclEventContent}, - StateEventType, + StateEventType, TimelineEventType, }, int, serde::Base64, @@ -796,7 +796,18 @@ impl Service { None::, |k, s| auth_events.get(&(k.clone(), s.to_owned())), ) - .map_err(|_e| Error::BadRequest(ErrorKind::InvalidParam, "Auth check failed."))?; + .map_err(|_e| Error::BadRequest(ErrorKind::InvalidParam, "Auth check failed."))? + || if let Some(redact_id) = &incoming_pdu.redacts { + incoming_pdu.kind == TimelineEventType::RoomRedaction + && !services().rooms.state_accessor.user_can_redact( + redact_id, + &incoming_pdu.sender, + &incoming_pdu.room_id, + true, + )? + } else { + false + }; // 13. Use state resolution to find new room state diff --git a/src/service/rooms/state_accessor/mod.rs b/src/service/rooms/state_accessor/mod.rs index c287edc5..8ca1b778 100644 --- a/src/service/rooms/state_accessor/mod.rs +++ b/src/service/rooms/state_accessor/mod.rs @@ -13,9 +13,11 @@ use ruma::{ history_visibility::{HistoryVisibility, RoomHistoryVisibilityEventContent}, member::{MembershipState, RoomMemberEventContent}, name::RoomNameEventContent, + power_levels::{RoomPowerLevels, RoomPowerLevelsEventContent}, }, StateEventType, }, + state_res::Event, EventId, JsOption, OwnedServerName, OwnedUserId, RoomId, ServerName, UserId, }; use serde_json::value::to_raw_value; @@ -351,4 +353,53 @@ impl Service { .map_err(|_| Error::bad_database("Invalid room member event in database.")) }) } + + /// Checks if a given user can redact a given event + /// + /// If federation is true, it allows redaction events from any user of the same server as the original event sender + pub fn user_can_redact( + &self, + redacts: &EventId, + sender: &UserId, + room_id: &RoomId, + federation: bool, + ) -> Result { + self.room_state_get(room_id, &StateEventType::RoomPowerLevels, "")? + .map(|e| { + serde_json::from_str(e.content.get()) + .map(|c: RoomPowerLevelsEventContent| c.into()) + .map(|e: RoomPowerLevels| { + e.user_can_redact_event_of_other(sender) + || e.user_can_redact_own_event(sender) + && if let Ok(Some(pdu)) = services().rooms.timeline.get_pdu(redacts) + { + if federation { + pdu.sender().server_name() == sender.server_name() + } else { + pdu.sender == sender + } + } else { + false + } + }) + .map_err(|_| { + Error::bad_database("Invalid m.room.power_levels event in database") + }) + }) + // Falling back on m.room.create to judge power levels + .unwrap_or_else(|| { + if let Some(pdu) = self.room_state_get(room_id, &StateEventType::RoomCreate, "")? { + Ok(pdu.sender == sender + || if let Ok(Some(pdu)) = services().rooms.timeline.get_pdu(redacts) { + pdu.sender == sender + } else { + false + }) + } else { + Err(Error::bad_database( + "No m.room.power_levels or m.room.create events in database for room", + )) + } + }) + } } diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs index 8f713c45..2752abe4 100644 --- a/src/service/rooms/timeline/mod.rs +++ b/src/service/rooms/timeline/mod.rs @@ -395,7 +395,14 @@ impl Service { | RoomVersionId::V9 | RoomVersionId::V10 => { if let Some(redact_id) = &pdu.redacts { - self.redact_pdu(redact_id, pdu)?; + if services().rooms.state_accessor.user_can_redact( + redact_id, + &pdu.sender, + &pdu.room_id, + false, + )? { + self.redact_pdu(redact_id, pdu)?; + } } } RoomVersionId::V11 => { @@ -405,7 +412,14 @@ impl Service { Error::bad_database("Invalid content in redaction pdu.") })?; if let Some(redact_id) = &content.redacts { - self.redact_pdu(redact_id, pdu)?; + if services().rooms.state_accessor.user_can_redact( + redact_id, + &pdu.sender, + &pdu.room_id, + false, + )? { + self.redact_pdu(redact_id, pdu)?; + } } } _ => unreachable!("Validity of room version already checked"), @@ -885,6 +899,23 @@ impl Service { } } + // If redaction event is not authorized, do not append it to the timeline + if let Some(redact_id) = &pdu.redacts { + if pdu.kind == TimelineEventType::RoomRedaction + && !services().rooms.state_accessor.user_can_redact( + redact_id, + &pdu.sender, + &pdu.room_id, + false, + )? + { + return Err(Error::BadRequest( + ErrorKind::Forbidden, + "User cannot redact this event.", + )); + } + }; + // We append to state before appending the pdu, so we don't have a moment in time with the // pdu without it's state. This is okay because append_pdu can't fail. let statehashid = services().rooms.state.append_to_state(&pdu)?;