Skip to content

Commit

Permalink
fixup! fix: Use the DisplayName struct to protect against homoglyph a…
Browse files Browse the repository at this point in the history
…ttacks
  • Loading branch information
poljar committed Nov 12, 2024
1 parent ea17551 commit 5a61e07
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 19 deletions.
15 changes: 15 additions & 0 deletions crates/matrix-sdk-base/src/deserialized_responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,21 @@ impl DisplayName {
self.decancered.as_ref().map(|d| d.as_str()).unwrap_or_else(|| &self.raw)
}

/// Returns the underlying raw and and unsanitized string of this
/// [`DisplayName`].
pub fn raw_str(&self) -> &str {
self.decancered.as_ref().map(|d| d.as_str()).unwrap_or_else(|| &self.raw)
}

/// Returns the underlying normalized and and sanitized string of this
/// [`DisplayName`].
///
/// Returns `None` if normalization failed during construction of this
/// [`DisplayName`].
pub fn as_normalized_str(&self) -> Option<&str> {
self.decancered.as_ref().map(|d| d.as_str())
}

fn has_cancer(&self) -> bool {
// If we managed to decancer the raw display name, and the decancered string is
// the same as the raw display name, then we're cancer free.
Expand Down
68 changes: 49 additions & 19 deletions crates/matrix-sdk-sqlite/src/state_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1294,13 +1294,27 @@ impl StateStore for SqliteStateStore {
let room_id = this.encode_key(keys::DISPLAY_NAME, room_id);

for (name, user_ids) in display_names {
let name = this.encode_key(keys::DISPLAY_NAME, name.as_str());
let encoded_name = this.encode_key(keys::DISPLAY_NAME, name.as_str());
let data = this.serialize_json(&user_ids)?;

if user_ids.is_empty() {
txn.remove_display_name(&room_id, &name)?;
txn.remove_display_name(&room_id, &encoded_name)?;

// This is a soft migration. See the comment in
// `get_users_with_display_names` for more info.
//
// `get_users_with_display_names` will fetch both the raw and normalized
// buckets and merge the user IDs.
//
// The SDK will then operate on then the merged buckets and update the
// user IDs that are using this display name.
// If the merged bucket is empty, we can remove both the raw and
// normalized bucket.
let raw_name = this.encode_key(keys::DISPLAY_NAME, name.raw_str());
txn.remove_display_name(&room_id, &raw_name)?;
} else {
txn.set_display_name(&room_id, &name, &data)?;
// We only create new buckets with the normalized display name.
txn.set_display_name(&room_id, &encoded_name, &data)?;
}
}
}
Expand Down Expand Up @@ -1511,31 +1525,47 @@ impl StateStore for SqliteStateStore {
room_id: &RoomId,
display_names: &'a [DisplayName],
) -> Result<HashMap<&'a DisplayName, BTreeSet<OwnedUserId>>> {
let mut result = HashMap::new();

if display_names.is_empty() {
return Ok(HashMap::new());
return Ok(result);
}

let room_id = self.encode_key(keys::DISPLAY_NAME, room_id);
let mut names_map = display_names
.iter()
.map(|n| (self.encode_key(keys::DISPLAY_NAME, n.as_str()), n))
.flat_map(|display_name| {
// We encode the display name as the `raw_str()` and the normalized string.
//
// This is for compatibility reasons since:
// 1. Previously "Alice" and "alice" were considered to be distinct display
// names, while we now consider them to be the same so we need to merge the
// previously distinct buckets of user IDs.
// 2. We can't do a migration to merge the previously distinct buckets of user
// IDs since the dipslay names itself are hashed before they are persisted
// in the store.
let raw =
(self.encode_key(keys::DISPLAY_NAME, display_name.raw_str()), display_name);
let normalized = display_name.as_normalized_str().map(|normalized| {
(self.encode_key(keys::DISPLAY_NAME, normalized), display_name)
});

iter::once(raw).chain(normalized.into_iter())
})
.collect::<BTreeMap<_, _>>();
let names = names_map.keys().cloned().collect();

self.acquire()
.await?
.get_display_names(room_id, names)
.await?
.into_iter()
.map(|(name, data)| {
Ok((
names_map
.remove(name.as_slice())
.expect("returned display names were requested"),
self.deserialize_json(&data)?,
))
})
.collect::<Result<HashMap<_, _>>>()
for (name, data) in
self.acquire().await?.get_display_names(room_id, names).await?.into_iter()
{
let display_name =
names_map.remove(name.as_slice()).expect("returned display names were requested");
let user_ids: BTreeSet<_> = self.deserialize_json(&data)?;

result.entry(display_name).or_insert_with(BTreeSet::new).extend(user_ids);
}

Ok(result)
}

async fn get_account_data_event(
Expand Down

0 comments on commit 5a61e07

Please sign in to comment.