diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 000000000..623cbd650 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-17 - XSS Vulnerability in Event Previews +**Vulnerability:** User-controlled fields like room aliases, canonical alias, custom guest access, and custom join rules were directly injected into text previews without HTML escaping when `format_as_html` was true in `src/event_preview.rs`. +**Learning:** The existing codebase lacked a systematic escaping mechanism for dynamic preview content, allowing user inputs to break out of HTML rendering contexts (e.g., Makepad `show_html`). This happened because formatting was done directly using `format!` and `String::push_str` with raw inputs. +**Prevention:** When injecting strings into HTML contexts, always use `htmlize::escape_text`. For fields conditionally rendered as HTML (returning `Cow<'_, str>`), explicitly use `Cow::Owned(htmlize::escape_text(input).into_owned())` to ensure both arms return identical types. diff --git a/src/event_preview.rs b/src/event_preview.rs index be1581e6b..e19848989 100644 --- a/src/event_preview.rs +++ b/src/event_preview.rs @@ -453,7 +453,11 @@ 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()); + if format_as_html { + s.push_str(&htmlize::escape_text(alias.as_str())); + } else { + s.push_str(alias.as_str()); + } if i != last_alias { s.push_str(", "); } @@ -465,9 +469,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 = if format_as_html { + Cow::Owned(htmlize::escape_text(alias).into_owned()) + } else { + Cow::Borrowed(alias) + }; + Some(format!("set the main address of this room to {}.", alias)) } AnyOtherStateEventContentChange::RoomCreate(StateEventContentChange::Original { content, .. }) => { Some(format!("created this room (v{}).", content.room_version.as_str())) @@ -479,17 +487,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 { + Cow::Owned(htmlize::escape_text(custom.as_str()).into_owned()) + } 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 { + Cow::Owned(htmlize::escape_text(custom.as_str()).into_owned()) + } else { + Cow::Borrowed(custom.as_str()) + }, }, )) } @@ -501,7 +520,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 { + Cow::Owned(htmlize::escape_text(custom.as_str()).into_owned()) + } else { + Cow::Borrowed(custom.as_str()) + }; + format!("set custom join rules for this room: {}", custom_str) + } }) } AnyOtherStateEventContentChange::RoomPinnedEvents(StateEventContentChange::Original { content, .. }) => {