fix: Use the DisplayName struct to protect against homoglyph attacks

Author: poljar

crates/matrix-sdk-base/src/client.rs

@@ -16,7 +16,7 @@
 #[cfg(feature = "e2e-encryption")]
 use std::sync::Arc;
 use std::{
-    collections::{BTreeMap, BTreeSet},
+    collections::{BTreeMap, BTreeSet, HashMap},
     fmt, iter,
     ops::Deref,
 };
@@ -70,7 +70,7 @@ use crate::latest_event::{is_suitable_for_latest_event, LatestEvent, PossibleLat
 #[cfg(feature = "e2e-encryption")]
 use crate::RoomMemberships;
 use crate::{
-    deserialized_responses::{RawAnySyncOrStrippedTimelineEvent, SyncTimelineEvent},
+    deserialized_responses::{DisplayName, RawAnySyncOrStrippedTimelineEvent, SyncTimelineEvent},
     error::{Error, Result},
     event_cache_store::EventCacheStoreLock,
     response_processors::AccountDataProcessor,
@@ -1332,7 +1332,7 @@ impl BaseClient {
         #[cfg(feature = "e2e-encryption")]
         let mut user_ids = BTreeSet::new();
 
-        let mut ambiguity_map: BTreeMap<String, BTreeSet<OwnedUserId>> = BTreeMap::new();
+        let mut ambiguity_map: HashMap<DisplayName, BTreeSet<OwnedUserId>> = Default::default();
 
         for raw_event in &response.chunk {
             let member = match raw_event.deserialize() {
@@ -1363,7 +1363,11 @@ impl BaseClient {
 
             if let StateEvent::Original(e) = &member {
                 if let Some(d) = &e.content.displayname {
-                    ambiguity_map.entry(d.clone()).or_default().insert(member.state_key().clone());
+                    let display_name = DisplayName::new(d);
+                    ambiguity_map
+                        .entry(display_name)
+                        .or_default()
+                        .insert(member.state_key().clone());
                 }
             }
 

crates/matrix-sdk-base/src/deserialized_responses.rs

@@ -446,10 +446,12 @@ impl MemberEvent {
     ///
     /// It there is no `displayname` in the event's content, the localpart or
     /// the user ID is returned.
-    pub fn display_name(&self) -> &str {
-        self.original_content()
-            .and_then(|c| c.displayname.as_deref())
-            .unwrap_or_else(|| self.user_id().localpart())
+    pub fn display_name(&self) -> DisplayName {
+        DisplayName::new(
+            self.original_content()
+                .and_then(|c| c.displayname.as_deref())
+                .unwrap_or_else(|| self.user_id().localpart()),
+        )
     }
 }
 

crates/matrix-sdk-base/src/rooms/members.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::{
-    collections::{BTreeMap, BTreeSet},
+    collections::{BTreeSet, HashMap},
     sync::Arc,
 };
 
@@ -30,7 +30,8 @@ use ruma::{
 };
 
 use crate::{
-    deserialized_responses::{MemberEvent, SyncOrStrippedState},
+    deserialized_responses::{DisplayName, MemberEvent, SyncOrStrippedState},
+    store::ambiguity_map::is_display_name_ambiguous,
     MinimalRoomMemberEvent,
 };
 
@@ -67,8 +68,10 @@ impl RoomMember {
         } = room_info;
 
         let is_room_creator = room_creator.as_deref() == Some(event.user_id());
-        let display_name_ambiguous =
-            users_display_names.get(event.display_name()).is_some_and(|s| s.len() > 1);
+        let display_name = event.display_name();
+        let display_name_ambiguous = users_display_names
+            .get(&display_name)
+            .is_some_and(|s| is_display_name_ambiguous(&display_name, s));
         let is_ignored = ignored_users.as_ref().is_some_and(|s| s.contains(event.user_id()));
 
         Self {
@@ -245,6 +248,6 @@ pub(crate) struct MemberRoomInfo<'a> {
     pub(crate) power_levels: Arc<Option<SyncOrStrippedState<RoomPowerLevelsEventContent>>>,
     pub(crate) max_power_level: i64,
     pub(crate) room_creator: Option<OwnedUserId>,
-    pub(crate) users_display_names: BTreeMap<&'a str, BTreeSet<OwnedUserId>>,
+    pub(crate) users_display_names: HashMap<&'a DisplayName, BTreeSet<OwnedUserId>>,
     pub(crate) ignored_users: Option<BTreeSet<OwnedUserId>>,
 }

crates/matrix-sdk-base/src/rooms/normal.rs

@@ -67,7 +67,7 @@ use super::{
 #[cfg(feature = "experimental-sliding-sync")]
 use crate::latest_event::LatestEvent;
 use crate::{
-    deserialized_responses::{MemberEvent, RawSyncOrStrippedState},
+    deserialized_responses::{DisplayName, MemberEvent, RawSyncOrStrippedState},
     notification_settings::RoomNotificationMode,
     read_receipts::RoomReadReceipts,
     store::{DynStateStore, Result as StoreResult, StateStoreExt},
@@ -819,8 +819,7 @@ impl Room {
             })
             .collect::<BTreeMap<_, _>>();
 
-        let display_names =
-            member_events.iter().map(|e| e.display_name().to_owned()).collect::<Vec<_>>();
+        let display_names = member_events.iter().map(|e| e.display_name()).collect::<Vec<_>>();
         let room_info = self.member_room_info(&display_names).await?;
 
         let mut members = Vec::new();
@@ -900,7 +899,7 @@ impl Room {
 
         let profile = self.store.get_profile(self.room_id(), user_id).await?;
 
-        let display_names = [event.display_name().to_owned()];
+        let display_names = [event.display_name()];
         let room_info = self.member_room_info(&display_names).await?;
 
         Ok(Some(RoomMember::from_parts(event, profile, presence, &room_info)))
@@ -911,7 +910,7 @@ impl Room {
     /// Async because it can read from storage.
     async fn member_room_info<'a>(
         &self,
-        display_names: &'a [String],
+        display_names: &'a [DisplayName],
     ) -> StoreResult<MemberRoomInfo<'a>> {
         let max_power_level = self.max_power_level();
         let room_creator = self.inner.read().creator().map(ToOwned::to_owned);

crates/matrix-sdk-base/src/sliding_sync/mod.rs

@@ -695,6 +695,8 @@ async fn cache_latest_events(
     changes: Option<&StateChanges>,
     store: Option<&Store>,
 ) {
+    use crate::{deserialized_responses::DisplayName, store::ambiguity_map::is_display_name_ambiguous};
+
     let mut encrypted_events =
         Vec::with_capacity(room.latest_encrypted_events.read().unwrap().capacity());
 
@@ -752,11 +754,13 @@ async fn cache_latest_events(
                                 .as_original()
                                 .and_then(|profile| profile.content.displayname.as_ref())
                                 .and_then(|display_name| {
+                                    let display_name = DisplayName::new(&display_name);
+
                                     changes.ambiguity_maps.get(room.room_id()).and_then(
                                         |map_for_room| {
                                             map_for_room
-                                                .get(display_name)
-                                                .map(|user_ids| user_ids.len() > 1)
+                                                .get(&display_name)
+                                                .map(|users| is_display_name_ambiguous(&display_name, users))
                                         },
                                     )
                                 });