From 7f46d9e2d5ce94a5dd0b2f394c9e0a233124cf6f Mon Sep 17 00:00:00 2001 From: DanS Date: Thu, 16 Jul 2026 16:07:12 -0500 Subject: [PATCH] fix(chat): harden reply-target + scrub legacy secret (B2/B4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B4: the legacy chat-identity fallback copied the z_exportkey spending key out of the RPC response and let the temporary json destruct un-zeroed. Mirror the mnemonic path — take our copy, then sodium_memzero the json's own buffer. B2: the memo header's peer z-address / cid ride OUTSIDE the secretstream AEAD, so trusting the newest message's header let a later message redirect our replies or splice threads. Pin the reply target (and displayed peer) to the EARLIEST (establishing) message instead of the latest, at both the send and display sites. Because ChatStore returned filtered INSERTION order (a scan harvests txids in set/hash order — not chronological), "earliest" wasn't reliable; ChatStore:: conversation now returns messages sorted by (timestamp, txid, payload_position), which also fixes out-of-order thread rendering and the last-message preview. A complete fix binds z+cid into the AEAD additional-data, but that's a coordinated HushChat/SDXLite wire-format change; this pin hardens the reply target without it. Adversarially verified; the store-ordering gap it surfaced is fixed here. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app_network.cpp | 20 ++++++++++++++++---- src/chat/chat_store.cpp | 12 ++++++++++++ src/ui/windows/chat_tab.cpp | 6 +++--- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/app_network.cpp b/src/app_network.cpp index 087a11e..fb4ccbc 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -2772,7 +2772,14 @@ void App::maybeProvisionChatIdentity() unavailable = true; } else { try { - secret = rpc_->call("z_exportkey", {fallbackZaddr}).get(); + // Mirror the mnemonic path: take our copy, then scrub the json's own copy of the + // spending key so it isn't left in freed heap (B4). z_exportkey returns a bare string. + auto keyResp = rpc_->call("z_exportkey", {fallbackZaddr}); + if (keyResp.is_string()) { + auto& key = keyResp.get_ref(); + secret = key; + if (!key.empty()) sodium_memzero(&key[0], key.size()); + } } catch (const rpc::RpcError&) { unavailable = true; // no spending key either (view-only?) — give up } catch (const std::exception&) { @@ -2921,12 +2928,17 @@ void App::sendChatMessage(const std::string& conversationId, const std::string& if (!chat::hushChatFeatureEnabledAtBuild() || !chat_service_.hasIdentity()) return; if (text.empty() || conversationId.empty()) return; - // The peer's z-address + public key come from a message we already have in this conversation. + // The peer's z-address + public key come from a message we already have in this conversation. Pin to + // the EARLIEST (establishing) values, not the latest: the memo header's `z`/`cid` ride outside the + // AEAD, so trusting the newest would let a later message redirect our replies to an attacker-chosen + // address / splice threads (B2). A full fix binds z+cid into the secretstream additional-data, but + // that's a coordinated HushChat/SDXLite wire-format change; pinning hardens the reply target without it. std::string peerZaddr; std::string peerPubKey; for (const auto& m : chat_service_.store().conversation(conversationId)) { - if (!m.peer_zaddr.empty()) peerZaddr = m.peer_zaddr; - if (!m.peer_public_key_hex.empty()) peerPubKey = m.peer_public_key_hex; + if (peerZaddr.empty() && !m.peer_zaddr.empty()) peerZaddr = m.peer_zaddr; + if (peerPubKey.empty() && !m.peer_public_key_hex.empty()) peerPubKey = m.peer_public_key_hex; + if (!peerZaddr.empty() && !peerPubKey.empty()) break; } if (peerPubKey.empty()) { ui::Notifications::instance().info(TR("chat_toast_waiting_reply")); diff --git a/src/chat/chat_store.cpp b/src/chat/chat_store.cpp index e43218d..3dbb0bd 100644 --- a/src/chat/chat_store.cpp +++ b/src/chat/chat_store.cpp @@ -2,6 +2,8 @@ #include "chat_store.h" +#include + namespace dragonx::chat { std::string ChatStore::dedupKey(const ChatMessage& message) { @@ -19,6 +21,16 @@ std::vector ChatStore::conversation(const std::string& conversation for (const auto& message : messages_) { if (message.conversation_id == conversationId) out.push_back(message); } + // Return chronological (oldest→newest). messages_ is in scan/insertion order, which is NOT + // time-ordered (a full scan harvests txids in std::set/unordered_map order) — that would mis-order the + // rendered thread AND let the reply-target pin (first-seen peer address) latch onto a non-establishing + // message (B2). timestamp is the block/tx time (peer can't set it); tie-break txid + payload_position + // for determinism. + std::stable_sort(out.begin(), out.end(), [](const ChatMessage& a, const ChatMessage& b) { + if (a.timestamp != b.timestamp) return a.timestamp < b.timestamp; + if (a.txid != b.txid) return a.txid < b.txid; + return a.payload_position < b.payload_position; + }); return out; } diff --git a/src/ui/windows/chat_tab.cpp b/src/ui/windows/chat_tab.cpp index e6f95ef..f110b7b 100644 --- a/src/ui/windows/chat_tab.cpp +++ b/src/ui/windows/chat_tab.cpp @@ -144,9 +144,9 @@ void RenderChatTab(App* app) ConvSummary c; c.cid = cid; c.count = static_cast(messages.size()); - for (const auto& m : messages) { // last non-empty peer z-addr / key across the thread - if (!m.peer_zaddr.empty()) c.peerZaddr = m.peer_zaddr; - if (!m.peer_public_key_hex.empty()) c.peerPubKey = m.peer_public_key_hex; + for (const auto& m : messages) { // pin to the EARLIEST (establishing) peer z-addr / key (B2 — the + if (c.peerZaddr.empty() && !m.peer_zaddr.empty()) c.peerZaddr = m.peer_zaddr; // memo header rides + if (c.peerPubKey.empty() && !m.peer_public_key_hex.empty()) c.peerPubKey = m.peer_public_key_hex; // outside the AEAD) } const auto& last = messages.back(); c.lastBody = last.body;