fix(bindings/cryto-nodejs): Fix memory corruption in async functions.

In async functions, the Node.js GC may or may not (that's a random
behavior) collect the arguments passed to the function as soon as it
returns. The function may not be executed yet, since it's async. Thus,
it leads to memory corruption: The function tries to read later on the
value inside an argument and… it crashes at best.

To avoid this bug, there is no other choice than cloning the values
before the function returns, in its “sync path” (so before any
transformation of an `.await` point into an “async block”).

The performance impact is not “massive”, I'm not sure it could be
noticeable easily since it is most of the time related to identifiers
(e.g. `UserId`), which are cheap to clone. I have to find the balance
here, and cloning offers the best trade off from my point of view.
This commit is contained in:
Ivan Enderlin
2022-07-05 08:59:11 +02:00
parent 81f02f0d0b
commit 607d7ebc22
2 changed files with 23 additions and 15 deletions

View File

@@ -59,10 +59,6 @@ impl UserId {
}
}
pub(crate) fn lower_user_ids_to_ruma(users: Vec<&UserId>) -> impl Iterator<Item = &ruma::UserId> {
users.into_iter().map(|user| user.inner.as_ref())
}
/// A Matrix device ID.
///
/// Device identifiers in Matrix are completely opaque character

View File

@@ -63,6 +63,9 @@ impl OlmMachine {
store_path: Option<String>,
mut store_passphrase: Option<String>,
) -> napi::Result<OlmMachine> {
let user_id = user_id.clone();
let device_id = device_id.clone();
let store = store_path
.map(|store_path| {
matrix_sdk_sled::CryptoStore::open_with_passphrase(
@@ -150,7 +153,7 @@ impl OlmMachine {
unused_fallback_keys: Vec<String>,
) -> napi::Result<String> {
let to_device_events = serde_json::from_str(to_device_events.as_ref()).map_err(into_err)?;
let changed_devices = &changed_devices.inner;
let changed_devices = changed_devices.inner.clone();
let one_time_key_counts = one_time_key_counts
.iter()
.map(|(key, value)| (DeviceKeyAlgorithm::from(key.as_str()), UInt::from(*value)))
@@ -167,7 +170,7 @@ impl OlmMachine {
.inner
.receive_sync_changes(
to_device_events,
changed_devices,
&changed_devices,
&one_time_key_counts,
unused_fallback_keys.as_deref(),
)
@@ -275,9 +278,15 @@ impl OlmMachine {
&self,
users: Option<Vec<&identifiers::UserId>>,
) -> napi::Result<Option<requests::KeysClaimRequest>> {
let users = users
.unwrap_or_default()
.into_iter()
.map(|user| user.inner.clone())
.collect::<Vec<_>>();
match self
.inner
.get_missing_sessions(identifiers::lower_user_ids_to_ruma(users.unwrap_or_default()))
.get_missing_sessions(users.iter().map(AsRef::as_ref))
.await
.map_err(into_err)?
{
@@ -306,7 +315,9 @@ impl OlmMachine {
/// * `users`, an array over user IDs that should be marked for tracking.
#[napi]
pub async fn update_tracked_users(&self, users: Vec<&identifiers::UserId>) {
self.inner.update_tracked_users(identifiers::lower_user_ids_to_ruma(users)).await;
let users = users.into_iter().map(|user| user.inner.clone()).collect::<Vec<_>>();
self.inner.update_tracked_users(users.iter().map(AsRef::as_ref)).await;
}
/// Get to-device requests to share a room key with users in a room.
@@ -323,15 +334,15 @@ impl OlmMachine {
users: Vec<&identifiers::UserId>,
encryption_settings: &encryption::EncryptionSettings,
) -> napi::Result<String> {
let room_id = room_id.inner.as_ref();
let users = identifiers::lower_user_ids_to_ruma(users);
let room_id = room_id.inner.clone();
let users = users.into_iter().map(|user| user.inner.clone()).collect::<Vec<_>>();
let encryption_settings =
matrix_sdk_crypto::olm::EncryptionSettings::from(encryption_settings);
serde_json::to_string(
&self
.inner
.share_room_key(room_id, users, encryption_settings)
.share_room_key(&room_id, users.iter().map(AsRef::as_ref), encryption_settings)
.await
.map_err(into_err)?,
)
@@ -354,13 +365,13 @@ impl OlmMachine {
event_type: String,
content: String,
) -> napi::Result<String> {
let room_id = room_id.inner.as_ref();
let room_id = room_id.inner.clone();
let content: JsonValue = serde_json::from_str(content.as_str()).map_err(into_err)?;
serde_json::to_string(
&self
.inner
.encrypt_room_event_raw(room_id, content, event_type.as_ref())
.encrypt_room_event_raw(&room_id, content, event_type.as_ref())
.await
.map_err(into_err)?,
)
@@ -381,8 +392,9 @@ impl OlmMachine {
) -> napi::Result<responses::DecryptedRoomEvent> {
let event: OriginalSyncRoomEncryptedEvent =
serde_json::from_str(event.as_str()).map_err(into_err)?;
let room_id = room_id.inner.as_ref();
let room_event = self.inner.decrypt_room_event(&event, room_id).await.map_err(into_err)?;
let room_id = room_id.inner.clone();
let room_event = self.inner.decrypt_room_event(&event, &room_id).await.map_err(into_err)?;
Ok(room_event.into())
}