CVE-2022-39264: Improper Authentication (CWE-287)
value: nheko is a desktop client for the Matrix communication application. All versions below 0.10.2 are vulnerable homeservers inserting malicious secrets, which could lead to man-in-the-middle attacks. Users can upgrade to version 0.10.2 to protect against this issue. As a workaround, one may apply the patch manually, avoid doing verifications of one's own devices, and/or avoid pressing the request button in the settings menu.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 5.9
Published: 2022-09-28T22:15Z
Repository: https://github.com/Nheko-Reborn/nheko
Language: C++
Files changed: src/encryption/Olm.cpp
Commit date: 2022-09-28 13:36:52 +0200
Commit message: Prevent the homeserver from inserting malicious secrets

Correctly verify that the reply to a secrets request is actually coming
from a verified device. While we did verify that it was us who replied,
we didn't properly cancel storing the secret if the sending device was
one of ours but was maliciously inserted by the homeserver and
unverified. We only send secret requests to verified devices in the
first place, so only the homeserver could abuse this issue.

Additionally we protected against malicious secret poisoning by
verifying that the secret is actually the reply to a request. This means
the server only has 2 places where it can poison the secrets:

- After a verification when we automatically request the secrets
- When the user manually hits the request button

It also needs to prevent other secret answers to reach the client first
since we ignore all replies after that one.

The impact of this might be quite severe. It could allow the server to
replace the cross-signing keys silently and while we might not trust
that key, we possibly could trust it in the future if we rely on the
stored secret. Similarly this could potentially be abused to make the
client trust a malicious online key backup.

If your deployment is not patched yet and you don't control your
homeserver, you can protect against this by simply not doing any
verifications of your own devices and not pressing the request button in
the settings menu.

Diff: diff --git a/src/encryption/Olm.cpp b/src/encryption/Olm.cpp
index b53d1ce4..6ab55b32 100644
--- a/src/encryption/Olm.cpp
+++ b/src/encryption/Olm.cpp
@@ -340,14 +340,17 @@ handle_olm_message(const OlmMessage &msg, const UserKeyCache &otherUserDeviceKey
                 auto local_user = http::client()->user_id();
 
                 if (msg.sender != local_user.to_string())
                     return;
 
-                auto secret_name = request_id_to_secret_name.find(e->content.request_id);
+                auto secret_name_it = request_id_to_secret_name.find(e->content.request_id);
 
-                if (secret_name != request_id_to_secret_name.end()) {
-                    nhlog::crypto()->info("Received secret: {}", secret_name->second);
+                if (secret_name_it != request_id_to_secret_name.end()) {
+                    auto secret_name = secret_name_it->second;
+                    request_id_to_secret_name.erase(secret_name_it);
+
+                    nhlog::crypto()->info("Received secret: {}", secret_name);
 
                     mtx::events::msg::SecretRequest secretRequest{};
                     secretRequest.action = mtx::events::msg::RequestAction::Cancellation;
                     secretRequest.requesting_device_id = http::client()->device_id();
                     secretRequest.request_id           = e->content.request_id;
@@ -356,19 +359,28 @@ handle_olm_message(const OlmMessage &msg, const UserKeyCache &otherUserDeviceKey
 
                     if (!verificationStatus)
                         return;
 
                     auto deviceKeys = cache::userKeys(local_user.to_string());
+                    if (!deviceKeys)
+                        return;
+
                     std::string sender_device_id;
-                    if (deviceKeys) {
-                        for (auto &[dev, key] : deviceKeys->device_keys) {
-                            if (key.keys["curve25519:" + dev] == msg.sender_key) {
-                                sender_device_id = dev;
-                                break;
-                            }
+                    for (auto &[dev, key] : deviceKeys->device_keys) {
+                        if (key.keys["curve25519:" + dev] == msg.sender_key) {
+                            sender_device_id = dev;
+                            break;
                         }
                     }
+                    if (!verificationStatus->verified_devices.count(sender_device_id) ||
+                        !verificationStatus->verified_device_keys.count(msg.sender_key) ||
+                        verificationStatus->verified_device_keys.at(msg.sender_key) !=
+                          crypto::Trust::Verified) {
+                        nhlog::net()->critical(
+                          "Received secret from unverified device {}! Ignoring!", sender_device_id);
+                        return;
+                    }
 
                     std::map<mtx::identifiers::User,
                              std::map<std::string, mtx::events::msg::SecretRequest>>
                       body;
 
@@ -378,23 +390,21 @@ handle_olm_message(const OlmMessage &msg, const UserKeyCache &otherUserDeviceKey
                     }
 
                     http::client()->send_to_device<mtx::events::msg::SecretRequest>(
                       http::client()->generate_txn_id(),
                       body,
-                      [name = secret_name->second](mtx::http::RequestErr err) {
+                      [secret_name](mtx::http::RequestErr err) {
                           if (err) {
                               nhlog::net()->error("Failed to send request cancellation "
                                                   "for secrect "
                                                   "'{}'",
-                                                  name);
+                                                  secret_name);
                           }
                       });
 
-                    nhlog::crypto()->info("Storing secret {}", secret_name->second);
-                    cache::client()->storeSecret(secret_name->second, e->content.secret);
-
-                    request_id_to_secret_name.erase(secret_name);
+                    nhlog::crypto()->info("Storing secret {}", secret_name);
+                    cache::client()->storeSecret(secret_name, e->content.secret);
                 }
 
             } else if (auto sec_req = std::get_if<DeviceEvent<msg::SecretRequest>>(&device_event)) {
                 handle_secret_request(sec_req, msg.sender);
             }

Vulnerable code:                 auto secret_name = request_id_to_secret_name.find(e->content.request_id);
                if (secret_name != request_id_to_secret_name.end()) {
                    nhlog::crypto()->info("Received secret: {}", secret_name->second);
                    if (deviceKeys) {
                        for (auto &[dev, key] : deviceKeys->device_keys) {
                            if (key.keys["curve25519:" + dev] == msg.sender_key) {
                                sender_device_id = dev;
                                break;
                            }
                      [name = secret_name->second](mtx::http::RequestErr err) {
                                                  name);
                    nhlog::crypto()->info("Storing secret {}", secret_name->second);
                    cache::client()->storeSecret(secret_name->second, e->content.secret);

                    request_id_to_secret_name.erase(secret_name);
Fixed code:                 auto secret_name_it = request_id_to_secret_name.find(e->content.request_id);
                if (secret_name_it != request_id_to_secret_name.end()) {
                    auto secret_name = secret_name_it->second;
                    request_id_to_secret_name.erase(secret_name_it);

                    nhlog::crypto()->info("Received secret: {}", secret_name);
                    if (!deviceKeys)
                        return;

                    for (auto &[dev, key] : deviceKeys->device_keys) {
                        if (key.keys["curve25519:" + dev] == msg.sender_key) {
                            sender_device_id = dev;
                            break;
                    if (!verificationStatus->verified_devices.count(sender_device_id) ||
                        !verificationStatus->verified_device_keys.count(msg.sender_key) ||
                        verificationStatus->verified_device_keys.at(msg.sender_key) !=
                          crypto::Trust::Verified) {
                        nhlog::net()->critical(
                          "Received secret from unverified device {}! Ignoring!", sender_device_id);
                        return;
                    }
                      [secret_name](mtx::http::RequestErr err) {
                                                  secret_name);
                    nhlog::crypto()->info("Storing secret {}", secret_name);
                    cache::client()->storeSecret(secret_name, e->content.secret);