CVE-2024-40648: Insufficient Information (NVD-CWE-noinfo)
value: matrix-rust-sdk is an implementation of a Matrix client-server library in Rust. The `UserIdentity::is_verified()` method in the matrix-sdk-crypto crate before version 0.7.2 doesn't take into account the verification status of the user's own identity while performing the check and may as a result return a value contrary to what is implied by its name and documentation. If the method is used to decide whether to perform sensitive operations towards a user identity, a malicious homeserver could manipulate the outcome in order to make the identity appear trusted. This is not a typical usage of the method, which lowers the impact. The method itself is not used inside the `matrix-sdk-crypto` crate. The 0.7.2 release of the `matrix-sdk-crypto` crate includes a fix. All users are advised to upgrade. There are no known workarounds for this vulnerability.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-18T17:15Z
Repository: https://github.com/matrix-org/matrix-rust-sdk
Language: Rust
Files changed: crates/matrix-sdk-crypto/src/identities/user.rs, crates/matrix-sdk-crypto/src/machine.rs
Commit date: 2024-07-18 16:20:47 +0200
Commit message: crypto: Fix UserIdentity::is_verified to take into account our own identity

The `UserIdentity::is_verified()` method in the matrix-sdk-crypto crate
before version 0.7.2 doesn't take into account the verification status
of the user's own identity while performing the check and may as a result
return a value contrary to what is implied by its name and documentation.

This patch fixes this and adds a regression test.

The method itself is not used internally and as such has not a larger
impact.

Co-authored-by: Denis Kasak <dkasak@termina.org.uk>
Signed-off-by: Damir Jelić <poljar@termina.org.uk>

Diff: diff --git a/crates/matrix-sdk-crypto/src/identities/user.rs b/crates/matrix-sdk-crypto/src/identities/user.rs
index 3d7918a49..3182abd39 100644
--- a/crates/matrix-sdk-crypto/src/identities/user.rs
+++ b/crates/matrix-sdk-crypto/src/identities/user.rs
@@ -225,11 +225,15 @@ impl Deref for UserIdentity {
 }
 
 impl UserIdentity {
     /// Is this user identity verified.
     pub fn is_verified(&self) -> bool {
-        self.own_identity.as_ref().is_some_and(|o| o.is_identity_signed(&self.inner).is_ok())
+        self.own_identity.as_ref().is_some_and(|own_identity| {
+            // The identity of another user is verified iff our own identity is verified and
+            // if our own identity has signed the other user's identity.
+            own_identity.is_verified() && own_identity.is_identity_signed(&self.inner).is_ok()
+        })
     }
 
     /// Manually verify this user.
     ///
     /// This method will attempt to sign the user identity using our private
@@ -787,24 +791,29 @@ pub(crate) mod testing {
 pub(crate) mod tests {
     use std::{collections::HashMap, sync::Arc};
 
     use assert_matches::assert_matches;
     use matrix_sdk_test::async_test;
-    use ruma::{device_id, user_id};
+    use ruma::{device_id, user_id, UserId};
     use serde_json::{json, Value};
     use tokio::sync::Mutex;
 
     use super::{
         testing::{device, get_other_identity, get_own_identity},
         OwnUserIdentityData, UserIdentityData,
     };
     use crate::{
         identities::{manager::testing::own_key_query, Device},
+        machine::tests::{
+            get_machine_pair, mark_alice_identity_as_verified_test_helper,
+            setup_cross_signing_for_machine_test_helper,
+        },
         olm::{Account, PrivateCrossSigningIdentity},
-        store::{CryptoStoreWrapper, MemoryStore},
+        store::{Changes, CryptoStoreWrapper, MemoryStore},
         types::{CrossSigningKey, MasterPubkey, SelfSigningPubkey, Signatures, UserSigningPubkey},
         verification::VerificationMachine,
+        OlmMachine,
     };
 
     #[test]
     fn own_identity_create() {
         let user_id = user_id!("@example:localhost");
@@ -1015,6 +1024,86 @@ pub(crate) mod tests {
         assert_eq!(
             identity.filter_devices_to_request(devices, unknown_device_id),
             [second_device_id]
         );
     }
+
+    async fn get_machine_pair_with_signed_identities(
+        alice: &UserId,
+        bob: &UserId,
+    ) -> (OlmMachine, OlmMachine) {
+        let (alice, bob, _) = get_machine_pair(alice, bob, false).await;
+        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
+        mark_alice_identity_as_verified_test_helper(&alice, &bob).await;
+
+        (alice, bob)
+    }
+
+    #[async_test]
+    async fn test_other_user_is_verified_if_my_identity_is_verified_and_they_are_cross_signed() {
+        let alice_user_id = user_id!("@alice:localhost");
+        let bob_user_id = user_id!("@bob:localhost");
+        let (alice, bob) =
+            get_machine_pair_with_signed_identities(alice_user_id, bob_user_id).await;
+
+        let bobs_own_identity =
+            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
+        let bobs_alice_identity =
+            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();
+
+        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
+        assert!(bobs_alice_identity.is_verified(), "Alice's identity should be verified as well.");
+    }
+
+    #[async_test]
+    async fn test_other_user_is_not_verified_if_they_are_not_cross_signed() {
+        let alice_user_id = user_id!("@alice:localhost");
+        let bob_user_id = user_id!("@bob:localhost");
+        let (alice, bob, _) = get_machine_pair(alice_user_id, bob_user_id, false).await;
+        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
+
+        let bobs_own_identity =
+            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
+        let bobs_alice_identity =
+            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();
+
+        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
+        assert!(!bobs_alice_identity.is_verified(), "Alice's identity should not be considered verified since Bob has not signed it.");
+    }
+
+    #[async_test]
+    async fn test_other_user_is_not_verified_if_my_identity_is_not_verified() {
+        let alice_user_id = user_id!("@alice:localhost");
+        let bob_user_id = user_id!("@bob:localhost");
+
+        let (alice, bob, _) = get_machine_pair(alice_user_id, bob_user_id, false).await;
+        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
+        mark_alice_identity_as_verified_test_helper(&alice, &bob).await;
+
+        let bobs_own_identity =
+            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
+        let bobs_alice_identity =
+            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();
+
+        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
+        assert!(bobs_alice_identity.is_verified(), "Alice's identity should be verified as well.");
+
+        bobs_own_identity.mark_as_unverified();
+
+        bob.store()
+            .save_changes(Changes {
+                identities: crate::store::IdentityChanges {
+                    changed: vec![bobs_own_identity.inner.clone().into()],
+                    ..Default::default()
+                },
+                ..Default::default()
+            })
+            .await
+            .unwrap();
+
+        assert!(!bobs_own_identity.is_verified(), "Bob's identity should not be verified anymore.");
+        assert!(
+            !bobs_alice_identity.is_verified(),
+            "Alice's identity should not be verified either."
+        );
+    }
 }
diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs
index b56fe14eb..7073f7394 100644
--- a/crates/matrix-sdk-crypto/src/machine.rs
+++ b/crates/matrix-sdk-crypto/src/machine.rs
@@ -3512,11 +3512,11 @@ pub(crate) mod tests {
             bob.get_room_event_encryption_info(&event, room_id).await,
             Err(MegolmError::JsonError(..))
         );
     }
 
-    async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
+    pub async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
         let CrossSigningBootstrapRequests { upload_signing_keys_req: alice_upload_signing, .. } =
             alice.bootstrap_cross_signing(false).await.expect("Expect Alice x-signing key request");
 
         let CrossSigningBootstrapRequests { upload_signing_keys_req: bob_upload_signing, .. } =
             bob.bootstrap_cross_signing(false).await.expect("Expect Bob x-signing key request");
@@ -3633,11 +3633,11 @@ pub(crate) mod tests {
 
         alice.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
         bob.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
     }
 
-    async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
+    pub async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
         let alice_device =
             bob.get_device(alice.user_id(), alice.device_id(), None).await.unwrap().unwrap();
 
         let alice_identity =
             bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();

Vulnerable code:         self.own_identity.as_ref().is_some_and(|o| o.is_identity_signed(&self.inner).is_ok())
    use ruma::{device_id, user_id};
        store::{CryptoStoreWrapper, MemoryStore},
    async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
    async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
Fixed code:         self.own_identity.as_ref().is_some_and(|own_identity| {
            // The identity of another user is verified iff our own identity is verified and
            // if our own identity has signed the other user's identity.
            own_identity.is_verified() && own_identity.is_identity_signed(&self.inner).is_ok()
        })
    use ruma::{device_id, user_id, UserId};
        machine::tests::{
            get_machine_pair, mark_alice_identity_as_verified_test_helper,
            setup_cross_signing_for_machine_test_helper,
        },
        store::{Changes, CryptoStoreWrapper, MemoryStore},
        OlmMachine,

    async fn get_machine_pair_with_signed_identities(
        alice: &UserId,
        bob: &UserId,
    ) -> (OlmMachine, OlmMachine) {
        let (alice, bob, _) = get_machine_pair(alice, bob, false).await;
        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
        mark_alice_identity_as_verified_test_helper(&alice, &bob).await;

        (alice, bob)
    }

    #[async_test]
    async fn test_other_user_is_verified_if_my_identity_is_verified_and_they_are_cross_signed() {
        let alice_user_id = user_id!("@alice:localhost");
        let bob_user_id = user_id!("@bob:localhost");
        let (alice, bob) =
            get_machine_pair_with_signed_identities(alice_user_id, bob_user_id).await;

        let bobs_own_identity =
            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
        let bobs_alice_identity =
            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();

        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
        assert!(bobs_alice_identity.is_verified(), "Alice's identity should be verified as well.");
    }

    #[async_test]
    async fn test_other_user_is_not_verified_if_they_are_not_cross_signed() {
        let alice_user_id = user_id!("@alice:localhost");
        let bob_user_id = user_id!("@bob:localhost");
        let (alice, bob, _) = get_machine_pair(alice_user_id, bob_user_id, false).await;
        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;

        let bobs_own_identity =
            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
        let bobs_alice_identity =
            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();

        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
        assert!(!bobs_alice_identity.is_verified(), "Alice's identity should not be considered verified since Bob has not signed it.");
    }

    #[async_test]
    async fn test_other_user_is_not_verified_if_my_identity_is_not_verified() {
        let alice_user_id = user_id!("@alice:localhost");
        let bob_user_id = user_id!("@bob:localhost");

        let (alice, bob, _) = get_machine_pair(alice_user_id, bob_user_id, false).await;
        setup_cross_signing_for_machine_test_helper(&alice, &bob).await;
        mark_alice_identity_as_verified_test_helper(&alice, &bob).await;

        let bobs_own_identity =
            bob.get_identity(bob.user_id(), None).await.unwrap().unwrap().own().unwrap();
        let bobs_alice_identity =
            bob.get_identity(alice.user_id(), None).await.unwrap().unwrap().other().unwrap();

        assert!(bobs_own_identity.is_verified(), "Bob's identity should be verified.");
        assert!(bobs_alice_identity.is_verified(), "Alice's identity should be verified as well.");

        bobs_own_identity.mark_as_unverified();

        bob.store()
            .save_changes(Changes {
                identities: crate::store::IdentityChanges {
                    changed: vec![bobs_own_identity.inner.clone().into()],
                    ..Default::default()
                },
                ..Default::default()
            })
            .await
            .unwrap();

        assert!(!bobs_own_identity.is_verified(), "Bob's identity should not be verified anymore.");
        assert!(
            !bobs_alice_identity.is_verified(),
            "Alice's identity should not be verified either."
        );
    }
    pub async fn setup_cross_signing_for_machine_test_helper(alice: &OlmMachine, bob: &OlmMachine) {
    pub async fn mark_alice_identity_as_verified_test_helper(alice: &OlmMachine, bob: &OlmMachine) {