From 7f47762e85a4b13889819cdc1f4c7e12e221451d Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 13 May 2026 04:29:24 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]=20Fi?= =?UTF-8?q?x=20XSS=20vulnerabilities=20in=20event=20previews?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sanitized user-controlled fields within `AnyOtherStateEventContentChange` and `OtherMessageLike` (like aliases, custom rules, and event types) in `src/event_preview.rs` when generating HTML previews. Co-authored-by: kevinaboos <1139460+kevinaboos@users.noreply.github.com> --- .jules/sentinel.md | 4 +++ src/event_preview.rs | 62 +++++++++++++++++++++++++++++++---------- src/home/room_screen.rs | 2 +- 3 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..90a7cd08 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-13 - XSS in Event Preview Formatting +**Vulnerability:** User-controlled content inside `AnyOtherStateEventContentChange` and `OtherMessageLike` (such as custom room aliases, canonical aliases, guest access rules, history visibility, and join rules) was directly embedded into `format!` strings without sanitization when generating HTML previews in `src/event_preview.rs`. +**Learning:** Rust's `format!` macro seamlessly handles standard string integration, making it easy to overlook that data dynamically generated by external users (like custom Matrix rules) still requires explicit HTML escaping when the output will be rendered via a rich text/HTML widget. +**Prevention:** Always trace the origin of fields within structs passed to preview formatters (like `event_type` and custom Matrix rules) and apply `htmlize::escape_text` when the `format_as_html` parameter is true. diff --git a/src/event_preview.rs b/src/event_preview.rs index be1581e6..58687991 100644 --- a/src/event_preview.rs +++ b/src/event_preview.rs @@ -98,7 +98,7 @@ pub fn text_preview_of_timeline_item( String::from("[Live Location]"), BeforeText::UsernameWithColon, )), - MsgLikeKind::Other(oml) => text_preview_of_other_message_like(oml), + MsgLikeKind::Other(oml) => text_preview_of_other_message_like(oml, true), } } TimelineItemContent::MembershipChange(membership_change) => { @@ -173,7 +173,7 @@ pub fn plaintext_body_of_timeline_item( String::from("[Live Location]") } MsgLikeKind::Other(other_msg_like) => { - text_preview_of_other_message_like(other_msg_like) + text_preview_of_other_message_like(other_msg_like, false) .format_with(&utils::get_or_fetch_event_sender(event_tl_item, None), false)} } } @@ -436,9 +436,16 @@ pub fn text_preview_of_encrypted_message( /// Returns a plaintext preview of the given other message-like event. pub fn text_preview_of_other_message_like( other_msg_like: &OtherMessageLike, + format_as_html: bool, ) -> TextPreview { + let event_type_owned = other_msg_like.event_type().to_string(); + let event_type_str = if format_as_html { + htmlize::escape_text(&event_type_owned) + } else { + Cow::Borrowed(event_type_owned.as_str()) + }; TextPreview::from(( - format!("[Other message type: {}]", other_msg_like.event_type()), + format!("[Other message type: {}]", event_type_str), BeforeText::UsernameWithColon, )) } @@ -453,7 +460,12 @@ pub fn text_preview_of_other_state( let mut s = String::from("set this room's aliases to "); let last_alias = content.aliases.len() - 1; for (i, alias) in content.aliases.iter().enumerate() { - s.push_str(alias.as_str()); + let alias_str = if format_as_html { + htmlize::escape_text(alias.as_str()) + } else { + Cow::Borrowed(alias.as_str()) + }; + s.push_str(&alias_str); if i != last_alias { s.push_str(", "); } @@ -465,9 +477,13 @@ pub fn text_preview_of_other_state( Some(String::from("set this room's avatar picture.")) } AnyOtherStateEventContentChange::RoomCanonicalAlias(StateEventContentChange::Original { content, .. }) => { - Some(format!("set the main address of this room to {}.", - content.alias.as_ref().map(|a| a.as_str()).unwrap_or("none") - )) + let alias = content.alias.as_ref().map(|a| a.as_str()).unwrap_or("none"); + let alias_str = if format_as_html { + htmlize::escape_text(alias) + } else { + Cow::Borrowed(alias) + }; + Some(format!("set the main address of this room to {}.", alias_str)) } AnyOtherStateEventContentChange::RoomCreate(StateEventContentChange::Original { content, .. }) => { Some(format!("created this room (v{}).", content.room_version.as_str())) @@ -479,17 +495,28 @@ pub fn text_preview_of_other_state( Some(match &content.guest_access { GuestAccess::CanJoin => String::from("has allowed guests to join this room."), GuestAccess::Forbidden => String::from("has forbidden guests from joining this room."), - custom => format!("has set custom guest access rules for this room: {}", custom.as_str()), + custom => { + let custom_str = if format_as_html { + htmlize::escape_text(custom.as_str()) + } else { + Cow::Borrowed(custom.as_str()) + }; + format!("has set custom guest access rules for this room: {}", custom_str) + } }) } AnyOtherStateEventContentChange::RoomHistoryVisibility(StateEventContentChange::Original { content, .. }) => { Some(format!("set this room's history to be visible by {}", match &content.history_visibility { - HistoryVisibility::Invited => "invited users, since they were invited.", - HistoryVisibility::Joined => "joined users, since they joined.", - HistoryVisibility::Shared => "joined users, for all of time.", - HistoryVisibility::WorldReadable => "anyone for all time.", - custom => custom.as_str(), + HistoryVisibility::Invited => Cow::Borrowed("invited users, since they were invited."), + HistoryVisibility::Joined => Cow::Borrowed("joined users, since they joined."), + HistoryVisibility::Shared => Cow::Borrowed("joined users, for all of time."), + HistoryVisibility::WorldReadable => Cow::Borrowed("anyone for all time."), + custom => if format_as_html { + htmlize::escape_text(custom.as_str()) + } else { + Cow::Borrowed(custom.as_str()) + }, }, )) } @@ -501,7 +528,14 @@ pub fn text_preview_of_other_state( JoinRule::Restricted(_) => String::from("set this room to be joinable by invite only or with restrictions."), JoinRule::KnockRestricted(_) => String::from("set this room to be joinable by invite only or requestable with restrictions."), JoinRule::Invite => String::from("set this room to be joinable by invite only."), - custom => format!("set custom join rules for this room: {}", custom.as_str()), + custom => { + let custom_str = if format_as_html { + htmlize::escape_text(custom.as_str()) + } else { + Cow::Borrowed(custom.as_str()) + }; + format!("set custom join rules for this room: {}", custom_str) + } }) } AnyOtherStateEventContentChange::RoomPinnedEvents(StateEventContentChange::Original { content, .. }) => { diff --git a/src/home/room_screen.rs b/src/home/room_screen.rs index d4a76a14..1969c299 100644 --- a/src/home/room_screen.rs +++ b/src/home/room_screen.rs @@ -4415,7 +4415,7 @@ impl SmallStateEventContent for OtherMessageLike { ) -> (WidgetRef, ItemDrawnStatus) { item.label(cx, ids!(content)).set_text( cx, - &text_preview_of_other_message_like(self).format_with(username, false), + &text_preview_of_other_message_like(self, false).format_with(username, false), ); new_drawn_status.content_drawn = true; (item, new_drawn_status)