DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

fix(chat,rpc): address adversarial review of the chat backlog

Browse Source 
Four confirmed findings from the review pass:

SECURITY — B7 was incomplete:
- The single-key Export dialog (key_export_dialog) still used plain call() for
  z_exportkey/dumpprivkey/z_exportviewingkey — a live spending-key leak on the
  most common per-address export path, missed by the B7 commit.
- callSecret() zeros the raw body but the parsed json holds its OWN heap copy of
  the secret; several callers did .get<string>() on a temporary json and freed
  that copy un-wiped.
  Fix: add RPCClient::callSecretString() — returns the bare-string result with
  BOTH the raw body AND the json node zeroed, so callers can't forget. Route
  key_export_dialog (×2), exportPrivateKey, and export_all_keys (×2) through it;
  scrub the z_exportmnemonic json node in seed_wallet_creator (object result);
  also wipe the transient key copies, the displayed s_key on reset, and the
  aggregated export-all `keys` buffer.

CHAT:
- Jump-to-latest pill: SetCursorScreenPos moved the parent cursor and never
  restored it, so the composer footer rendered ~8px too high while scrolled up.
  Save + restore the cursor around the pill.
- New-message toast: gating on a chatUnreadCount() watermark delta could be
  swallowed when an outgoing echo (wall-clock) pushed the seen-watermark past a
  later reply's block time. ingest() now reports the cids it appended; the toast
  fires when any is a non-muted conversation — skew-proof, still mute-aware.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-07-16 17:11:31 -05:00
parent 9cb91415d9
commit fac0245297
9 changed files with 99 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
src/app_network.cpp
View File

@@ -1689,11 +1689,14 @@ void App::refreshTransactionData()
!result.hushChatMetadata.empty()) {
std::unordered_map<std::string, std::int64_t> chatTxTimes;
for (const auto& tx : result.transactions) chatTxTimes[tx.txid] = tx.timestamp;
const int unreadBefore = chatUnreadCount();
const int newMsgs = chat_service_.ingest(result.hushChatMetadata, chatTxTimes, std::time(nullptr));
// Toast only when a non-muted conversation gained an unread message — chatUnreadCount()
// already skips muted cids, so a delta cleanly respects mute (Q10 + Q4).
if (newMsgs > 0 && current_page_ != ui::NavPage::Chat && chatUnreadCount() > unreadBefore)
std::vector<std::string> newChatCids;
chat_service_.ingest(result.hushChatMetadata, chatTxTimes, std::time(nullptr), &newChatCids);
// Toast when a NON-muted conversation received a new message and we're off the Chat tab.
// Gate on the ingest-reported cids, not a seen-watermark delta — the latter can be swallowed
// by block-time vs wall-clock (echo) skew (Q10 + Q4).
if (current_page_ != ui::NavPage::Chat &&
std::any_of(newChatCids.begin(), newChatCids.end(),
[this](const std::string& cid){ return !(settings_ && settings_->isChatMuted(cid)); }))
ui::Notifications::instance().info(TR("chat_new_message_toast"));
}
NetworkRefreshService::applyTransactionRefreshResult(
@@ -1752,11 +1755,14 @@ void App::refreshRecentTransactionData()
!result.hushChatMetadata.empty()) {
std::unordered_map<std::string, std::int64_t> chatTxTimes;
for (const auto& tx : result.transactions) chatTxTimes[tx.txid] = tx.timestamp;
const int unreadBefore = chatUnreadCount();
const int newMsgs = chat_service_.ingest(result.hushChatMetadata, chatTxTimes, std::time(nullptr));
// Toast only when a non-muted conversation gained an unread message — chatUnreadCount()
// already skips muted cids, so a delta cleanly respects mute (Q10 + Q4).
if (newMsgs > 0 && current_page_ != ui::NavPage::Chat && chatUnreadCount() > unreadBefore)
std::vector<std::string> newChatCids;
chat_service_.ingest(result.hushChatMetadata, chatTxTimes, std::time(nullptr), &newChatCids);
// Toast when a NON-muted conversation received a new message and we're off the Chat tab.
// Gate on the ingest-reported cids, not a seen-watermark delta — the latter can be swallowed
// by block-time vs wall-clock (echo) skew (Q10 + Q4).
if (current_page_ != ui::NavPage::Chat &&
std::any_of(newChatCids.begin(), newChatCids.end(),
[this](const std::string& cid){ return !(settings_ && settings_->isChatMuted(cid)); }))
ui::Notifications::instance().info(TR("chat_new_message_toast"));
}
NetworkRefreshService::applyTransactionRefreshResult(
@@ -2639,7 +2645,7 @@ void App::exportPrivateKey(const std::string& address, std::function<void(const
std::string err;
try {
rpc::RPCClient::TraceScope trace("Settings / Export private key");
key = rpc_->callSecret(method, {address}).get<std::string>(); // zero the raw body too (B7)
key = rpc_->callSecretString(method, {address}); // scrubs raw body + json node (B7)
} catch (const std::exception& e) {
err = e.what();
}

6
src/chat/chat_service.cpp
View File

@@ -25,7 +25,8 @@ void ChatService::clearIdentity() {
int ChatService::ingest(const std::vector<HushChatTransactionMetadata>& metadata,
const std::unordered_map<std::string, std::int64_t>& txTimestamps,
std::int64_t fallbackTimestamp) {
std::int64_t fallbackTimestamp,
std::vector<std::string>* newIncomingCids) {
if (!has_identity_) return 0;
int added = 0;
@@ -59,6 +60,9 @@ int ChatService::ingest(const std::vector<HushChatTransactionMetadata>& metadata
if (store_.append(message)) {
if (db_) db_->append(message);
++added;
// Every ingested message is incoming — report its cid so the caller can notify without
// relying on a seen-watermark delta (which block-time vs wall-clock skew can swallow).
if (newIncomingCids) newIncomingCids->push_back(message.conversation_id);
}
}
return added;

6
src/chat/chat_service.h
View File

@@ -40,9 +40,13 @@ public:
// when the txid isn't present. Newly-added messages are also persisted (if a database is
// attached). Returns the number of NEW messages added; 0 with no identity. Undecryptable
// Messages are dropped silently (no logging of memo/plaintext).
// When `newIncomingCids` is non-null it is filled with the conversation ids of the genuinely-new
// incoming (non-request) messages appended this call — a reliable "a new message arrived here"
// signal for notifications that doesn't depend on any timestamp/seen-watermark comparison.
int ingest(const std::vector<HushChatTransactionMetadata>& metadata,
const std::unordered_map<std::string, std::int64_t>& txTimestamps,
std::int64_t fallbackTimestamp = 0);
std::int64_t fallbackTimestamp = 0,
std::vector<std::string>* newIncomingCids = nullptr);
// Attach a persistent backing store (Phase 2). Not owned. Pass nullptr to detach. New messages
// from ingest() are written through; loadFromDatabase() rehydrates the in-memory store from it.

8
src/daemon/seed_wallet_creator.cpp
View File

@@ -111,8 +111,12 @@ SeedWalletResult SeedWalletCreator::create(bool keepDatadir,
// 6. Export the new seed phrase + a fresh shielded receive address (the future sweep target).
try {
auto m = cli.callSecret("z_exportmnemonic"); // zero the raw body too (B7)
if (m.contains("mnemonic") && m["mnemonic"].is_string())
r.seedPhrase = m["mnemonic"].get<std::string>();
if (m.contains("mnemonic") && m["mnemonic"].is_string()) {
// Take our copy, then scrub the json node's own copy so it isn't freed in the clear (B7).
auto& mn = m["mnemonic"].get_ref<std::string&>();
r.seedPhrase = mn;
if (!mn.empty()) sodium_memzero(&mn[0], mn.size());
}
r.destAddress = cli.call("z_getnewaddress").get<std::string>();
r.ok = !r.seedPhrase.empty() && !r.destAddress.empty();
if (!r.ok) r.error = "The isolated node returned an empty seed or address.";

29
src/rpc/rpc_client.cpp
View File

@@ -391,6 +391,35 @@ json RPCClient::callSecret(const std::string& method, const json& params)
}
}
std::string RPCClient::callSecretString(const std::string& method, const json& params)
{
std::lock_guard<std::recursive_mutex> lk(curl_mutex_);
if (!impl_->curl) {
throw std::runtime_error("Not connected");
}
long http_code = 0;
std::string response_data = performCall(method, params, http_code);
try {
json result = parseRpcResult(http_code, response_data);
std::string out;
if (result.is_string()) {
// Copy the secret out, then zero the json node's OWN heap buffer — parseRpcResult's
// json::parse allocates this independently of response_data, so scrubbing the raw body
// alone would leave it behind (B7). `out` is now the only live copy; the caller wipes it.
auto& s = result.get_ref<std::string&>();
out = s;
if (!s.empty()) sodium_memzero(&s[0], s.size());
}
if (!response_data.empty()) sodium_memzero(&response_data[0], response_data.size());
if (!result.is_string()) throw std::runtime_error("RPC result is not a string");
return out;
} catch (...) {
if (!response_data.empty()) sodium_memzero(&response_data[0], response_data.size());
throw;
}
}
json RPCClient::call(const std::string& method, const json& params, long timeoutSec)
{
std::lock_guard<std::recursive_mutex> lk(curl_mutex_);

9
src/rpc/rpc_client.h
View File

@@ -144,6 +144,15 @@ public:
*/
json callSecret(const std::string& method, const json& params = json::array());
/**
* @brief callSecret() for RPCs whose result is a bare secret string (z_exportkey / dumpprivkey /
* z_exportviewingkey). Returns the result string with BOTH the raw response body AND the
* parsed json's own copy of the secret zeroed — so no un-scrubbed heap copy survives the
* call. The returned std::string is the only remaining copy; the caller owns it and must
* wipe it (sodium_memzero) when done. Throws if the result is not a JSON string.
*/
std::string callSecretString(const std::string& method, const json& params = json::array());
/**
* @brief Make a raw RPC call with a custom timeout
* @param method RPC method name

6
src/ui/windows/chat_tab.cpp
View File

@@ -525,14 +525,18 @@ void RenderChatTab(App* app)
atBottom = ImGui::GetScrollY() >= ImGui::GetScrollMaxY() - 4.0f;
}
ImGui::EndChild();
// Jump-to-latest pill when scrolled up (Q9).
// Jump-to-latest pill when scrolled up (Q9). SetCursorScreenPos moves the parent cursor to
// the child's bottom edge; save + restore it so the composer footer below stays anchored
// (otherwise it renders ~8px too high only while the pill is shown).
if (!atBottom) {
const ImVec2 savedCursor = ImGui::GetCursorScreenPos();
const ImVec2 pill(msgWinMin.x + msgWinSize.x - 84.0f * dp, msgWinMin.y + msgWinSize.y - 34.0f * dp);
ImGui::SetCursorScreenPos(pill);
ImGui::PushStyleColor(ImGuiCol_Button, ImGui::ColorConvertU32ToFloat4(material::WithAlpha(material::Primary(), 220)));
ImGui::PushStyleColor(ImGuiCol_ButtonHovered, ImGui::ColorConvertU32ToFloat4(material::Primary()));
if (ImGui::Button(TR("chat_jump_latest"), ImVec2(74.0f * dp, 26.0f * dp))) s_scroll_to_cid = sel->cid;
ImGui::PopStyleColor(2);
ImGui::SetCursorScreenPos(savedCursor);
}
// Composer footer: message input + send (only once we know the peer's key), else a hint.

28
src/ui/windows/export_all_keys_dialog.cpp
View File

@@ -4,6 +4,7 @@
#include "export_all_keys_dialog.h"
#include "../../app.h"
#include <sodium.h> // sodium_memzero — wipe exported key material from memory (B7)
#include "../../rpc/rpc_client.h"
#include "../../rpc/rpc_worker.h"
#include "../../util/i18n.h"
@@ -180,13 +181,12 @@ void ExportAllKeysDialog::render(App* app)
for (const auto& addr : z_addrs) {
try {
rpc::RPCClient::TraceScope trace("Settings / Export all keys");
auto result = rpc->callSecret("z_exportkey", {addr}); // zero the raw body too (B7)
if (result.is_string()) {
keys += "# Address: " + addr + "\n";
keys += result.get<std::string>() + "\n\n";
exported++; // count only real successes (locked/failed keys don't count)
}
} catch (...) {}
std::string k = rpc->callSecretString("z_exportkey", {addr}); // scrubs raw body + json node (B7)
keys += "# Address: " + addr + "\n";
keys += k + "\n\n";
exported++; // count only real successes (locked/failed keys don't count)
if (!k.empty()) sodium_memzero(&k[0], k.size()); // wipe the transient copy
} catch (...) {} // non-string / locked key → skip (not counted)
}
}
@@ -196,13 +196,12 @@ void ExportAllKeysDialog::render(App* app)
for (const auto& addr : t_addrs) {
try {
rpc::RPCClient::TraceScope trace("Settings / Export all keys");
auto result = rpc->callSecret("dumpprivkey", {addr}); // zero the raw body too (B7)
if (result.is_string()) {
keys += "# Address: " + addr + "\n";
keys += result.get<std::string>() + "\n\n";
exported++; // count only real successes (locked/failed keys don't count)
}
} catch (...) {}
std::string k = rpc->callSecretString("dumpprivkey", {addr}); // scrubs raw body + json node (B7)
keys += "# Address: " + addr + "\n";
keys += k + "\n\n";
exported++; // count only real successes (locked/failed keys don't count)
if (!k.empty()) sodium_memzero(&k[0], k.size()); // wipe the transient copy
} catch (...) {} // non-string / locked key → skip (not counted)
}
}
@@ -219,6 +218,7 @@ void ExportAllKeysDialog::render(App* app)
writeOk = true;
}
}
if (!keys.empty()) sodium_memzero(&keys[0], keys.size()); // don't leave every key in freed heap
return [exported, total, filepath, writeOk]() {
s_exported_count = exported;

17
src/ui/windows/key_export_dialog.cpp
View File

@@ -5,6 +5,7 @@
#include "key_export_dialog.h"
#include "../../app.h"
#include "../../wallet/lite_wallet_controller.h"
#include <sodium.h> // sodium_memzero — wipe transient secret copies (B7)
#include <nlohmann/json.hpp>
#include "../../rpc/rpc_client.h"
#include "../../rpc/rpc_worker.h"
@@ -49,7 +50,7 @@ void KeyExportDialog::show(const std::string& address, KeyType type)
releaseQr();
s_key_type = type;
s_address = address;
s_key.clear();
wallet::secureWipeLiteSecret(s_key); // zero any prior secret before reuse
s_error.clear();
}
@@ -62,7 +63,7 @@ void KeyExportDialog::hide()
{
s_open = false;
s_fetching = false;
s_key.clear();
wallet::secureWipeLiteSecret(s_key); // zero the displayed secret, don't just drop the buffer
s_show_key = false;
s_error.clear();
releaseQr();
@@ -187,18 +188,18 @@ void KeyExportDialog::render(App* app)
std::string error;
try {
rpc::RPCClient::TraceScope trace("Settings / Export key");
auto result = rpc->call(method, {addr});
key = result.get<std::string>();
key = rpc->callSecretString(method, {addr}); // scrubs raw body + json node (B7)
} catch (const std::exception& e) {
error = e.what();
}
return [key, error]() {
return [key = std::move(key), error]() mutable {
if (error.empty()) {
s_key = key;
s_show_key = false; // Don't show by default
} else {
s_error = error;
}
if (!key.empty()) sodium_memzero(&key[0], key.size()); // wipe the last transient copy
s_fetching = false;
};
});
@@ -213,18 +214,18 @@ void KeyExportDialog::render(App* app)
std::string error;
try {
rpc::RPCClient::TraceScope trace("Settings / Export viewing key");
auto result = rpc->call("z_exportviewingkey", {addr});
key = result.get<std::string>();
key = rpc->callSecretString("z_exportviewingkey", {addr}); // scrubs raw body + json node (B7)
} catch (const std::exception& e) {
error = e.what();
}
return [key, error]() {
return [key = std::move(key), error]() mutable {
if (error.empty()) {
s_key = key;
s_show_key = true; // Viewing keys are less sensitive
} else {
s_error = error;
}
if (!key.empty()) sodium_memzero(&key[0], key.size()); // wipe the last transient copy
s_fetching = false;
};
});