From c0d10e9664cf5e9c6fbabe3a907e945bc13f20e5 Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 19 Oct 2026 12:05:04 -0500 Subject: [PATCH] security(hygiene): best-effort scrubbing of residual in-memory secret copies (Tier 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defense-in-depth against a local attacker / core dump reading un-zeroed heap copies of secrets (threat model is local, not remote). Best-effort by nature — std::string SSO/reallocation and ImGui buffer growth can still strand copies — so these shrink the exposure window rather than guarantee absence: - rpc_client performCall: a scope guard now sodium_memzeroes the request body + payload tree (walletpassphrase / *_importkey args) after the call and detaches CURLOPT_POSTFIELDS. Request content sent to curl is unchanged. - New ui::scrubImGuiInputTextState() zeroes ImGui's retained InputText buffers (live/revert/callback/deactivated) — the app memzeroed its own char[]s but ImGui kept copies. Called from wipeSecrets()/wipeDisplayedSecrets(). PINNED to ImGui 1.92.6 via a #error guard so a vendored bump forces a re-audit (the stb undo buffer is an incomplete type in the header, so it's left as a noted gap). - Lite exportSeed/exportPrivateKeys wipe result.value + the parsed JSON tree. - Seed-word grid (SplitSeedWords/RenderSeedWordGrid) zeroes its per-frame word and cell copies; the wizard word/chip vectors are zeroed before clearing. - ChatStore::clear() + ResetChatTab() zero decrypted message bodies on lock/switch/teardown. Co-Authored-By: Claude Opus 4.8 --- src/app.cpp | 19 +++++++--- src/chat/chat_store.cpp | 5 +++ src/rpc/rpc_client.cpp | 14 +++++++ src/ui/imgui_secret_scrub.h | 53 +++++++++++++++++++++++++++ src/ui/windows/chat_tab.cpp | 5 +++ src/ui/windows/seed_display.h | 19 +++++++++- src/wallet/lite_wallet_controller.cpp | 26 +++++++++++-- 7 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 src/ui/imgui_secret_scrub.h diff --git a/src/app.cpp b/src/app.cpp index 5fa0f68..28d6ca0 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -40,6 +40,7 @@ #include "ui/windows/faq_dialog.h" #include "embedded/IconsMaterialDesign.h" #include "ui/windows/key_export_dialog.h" +#include "ui/imgui_secret_scrub.h" // scrubImGuiInputTextState — zero ImGui's retained secret buffers #include "ui/windows/transaction_details_dialog.h" #include "ui/windows/qr_popup_dialog.h" #include "ui/windows/validate_address_dialog.h" @@ -143,6 +144,7 @@ void App::wipeSecrets() sodium_memzero(export_result_, sizeof(export_result_)); // exported WIF/z-key (SECRET) sodium_memzero(import_key_input_, sizeof(import_key_input_)); // pasted private key (SECRET) sodium_memzero(import_clip_preview_, sizeof(import_clip_preview_)); // clipboard preview (SECRET) + ui::scrubImGuiInputTextState(); // zero ImGui's retained copies of whatever secret was last typed } // Scrub + close any secret that is currently ON SCREEN (revealed seed phrase, migration seed, exported @@ -163,6 +165,7 @@ void App::wipeDisplayedSecrets() sodium_memzero(export_result_, sizeof(export_result_)); ui::KeyExportDialog::hide(); // zeros its static s_key via secureWipeLiteSecret console_tab_.wipeSecretResult(); // drop any masked key-dump result held in the console + ui::scrubImGuiInputTextState(); // zero ImGui's retained copy of the last-typed passphrase/PIN } // Enqueue a blocking xmrig start/stop op onto the dedicated serialized control thread so the render thread @@ -3486,14 +3489,20 @@ void App::renderLiteFirstRunPrompt() ImGui::SetNextWindowPos(ImVec2(vp->Pos.x + vp->Size.x * 0.5f, vp->Pos.y + vp->Size.y * 0.5f), ImGuiCond_Appearing, ImVec2(0.5f, 0.5f)); + // Best-effort zero of the seed-word statics (they hold the whole phrase) before they're dropped. + auto wipeStr = [](std::string& s) { if (!s.empty()) sodium_memzero(&s[0], s.size()); s.clear(); }; + auto wipeWords = [&](std::vector& v) { for (auto& w : v) wipeStr(w); v.clear(); }; + auto wipeChips = [&](std::vector>& v) { + for (auto& c : v) { wipeStr(c.first); c.second = false; } v.clear(); + }; auto finish = [&]() { wallet::secureWipeLiteSecret(seed); sodium_memzero(restoreSeed, sizeof(restoreSeed)); restoreBirthday = 0; restoring = false; restoreErr.clear(); - words.clear(); - chips.clear(); + wipeWords(words); + wipeChips(chips); progress = 0; step = 0; skipConfirm = false; @@ -3601,7 +3610,7 @@ void App::renderLiteFirstRunPrompt() ImGui::Spacing(); ImGui::Spacing(); if (ui::material::TactileButton(TR("appx_ive_written_it_down"), ImVec2(btnW, 0))) { - chips.clear(); + wipeChips(chips); for (const auto& w : words) chips.emplace_back(w, false); std::mt19937 rng{std::random_device{}()}; std::shuffle(chips.begin(), chips.end(), rng); @@ -4734,7 +4743,7 @@ void App::renderSeedBackupDialog() ImGui::Spacing(); ImGui::TextWrapped("%s", TR("seed_backup_intro")); ImGui::Spacing(); - ui::RenderSeedWordGrid(ui::SplitSeedWords(seed_backup_phrase_)); + { auto words = ui::SplitSeedWords(seed_backup_phrase_); ui::RenderSeedWordGrid(words); ui::WipeSeedWords(words); } ImGui::Spacing(); if (!seed_backup_status_.empty()) { ImGui::PushTextWrapPos(0.0f); @@ -4904,7 +4913,7 @@ void App::renderSeedMigrationDialog() // distinct artifact, not body copy. Purely visual — the words/derivation/logic are untouched. { ui::material::GlassSectionScope seedPanel; - ui::RenderSeedWordGrid(ui::SplitSeedWords(seed_migration_seed_)); + { auto words = ui::SplitSeedWords(seed_migration_seed_); ui::RenderSeedWordGrid(words); ui::WipeSeedWords(words); } } ImGui::Spacing(); ImGui::TextColored(kMedium, "%s", TR("mig_receive_addr")); diff --git a/src/chat/chat_store.cpp b/src/chat/chat_store.cpp index 8fc044f..5c92677 100644 --- a/src/chat/chat_store.cpp +++ b/src/chat/chat_store.cpp @@ -3,6 +3,7 @@ #include "chat_store.h" #include +#include namespace dragonx::chat { @@ -112,6 +113,10 @@ std::vector ChatStore::eraseConversation(const std::string& convers } void ChatStore::clear() { + // Zero decrypted plaintext (message bodies + peer address) before dropping them — clear() runs on + // wallet lock / switch / teardown, and the freed std::strings would otherwise linger in the heap. + auto wipe = [](std::string& s) { if (!s.empty()) sodium_memzero(&s[0], s.size()); s.clear(); }; + for (auto& m : messages_) { wipe(m.body); wipe(m.fail_reason); wipe(m.peer_zaddr); } messages_.clear(); seen_.clear(); txid_index_.clear(); // drop the History-badge txid copies too (revision_ below invalidates the memo) diff --git a/src/rpc/rpc_client.cpp b/src/rpc/rpc_client.cpp index 57d6e28..87805e8 100644 --- a/src/rpc/rpc_client.cpp +++ b/src/rpc/rpc_client.cpp @@ -370,6 +370,20 @@ std::string RPCClient::performCall(const std::string& method, const json& params json payload = makePayload(method, params); std::string body = payload.dump(); + // Request-side secret scrub (responses are already scrubbed): the body + payload tree can carry + // walletpassphrase / encryptwallet / *_importkey args. Wipe them when this scope ends (all return + // and throw paths) and detach CURLOPT_POSTFIELDS so the reused handle holds no dangling pointer. + struct RequestScrubGuard { + CURL* curl; std::string& body; json& payload; + ~RequestScrubGuard() { + if (!body.empty()) sodium_memzero(&body[0], body.size()); + scrubJsonSecrets(payload); + if (curl) { + curl_easy_setopt(curl, CURLOPT_POSTFIELDS, nullptr); + curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, 0L); + } + } + } requestScrubGuard{impl_->curl, body, payload}; std::string response_data; // Set POST data diff --git a/src/ui/imgui_secret_scrub.h b/src/ui/imgui_secret_scrub.h new file mode 100644 index 0000000..9d2accf --- /dev/null +++ b/src/ui/imgui_secret_scrub.h @@ -0,0 +1,53 @@ +// DragonX Wallet - ImGui Edition +// Copyright 2024-2026 The Hush Developers +// Released under the GPLv3 +#pragma once + +// Best-effort scrub of ImGui's internal InputText buffers, which retain un-zeroed copies of typed +// secrets (wallet passphrase / PIN / seed / private key) after a field is edited — the app's own +// char[] buffers are memzeroed, but ImGui also copies the text into its global edit/undo/deactivated +// state, which it only frees (not zeroes). +// +// This reaches into vendored imgui_internal state, so it is PINNED to the vendored ImGui version: a +// bump trips the #error below and forces a re-audit of the member names. It is BEST-EFFORT only — +// std::-style buffer reallocation, the system clipboard, and OS/IME layers can still hold copies, so +// this shrinks the exposure window but does not guarantee absence of secret material in a core dump. + +#include "imgui.h" +#include "imgui_internal.h" +#include +#include + +#if IMGUI_VERSION_NUM != 19259 +#error "imgui_secret_scrub.h is pinned to ImGui 1.92.6 (IMGUI_VERSION_NUM 19259). After bumping libs/imgui, re-audit ImGuiInputTextState / ImGuiInputTextDeactivatedState / ImGuiContext member names below, then update this guard." +#endif + +namespace dragonx { +namespace ui { + +// Zero ImGui's retained InputText buffers. Safe to call at a lock transition / when a secret dialog +// closes (it defocuses the active field). Do NOT call while a secret field is actively being typed. +inline void scrubImGuiInputTextState() { + ImGuiContext* ctx = ImGui::GetCurrentContext(); + if (!ctx) return; + ImGuiInputTextState& st = ctx->InputTextState; + // Defocus first so the next frame can't re-read the buffer we're about to zero (ClearActiveID also + // refreshes the deactivated-state copy, which we then zero below). + if (st.ID != 0 && ctx->ActiveId == st.ID) ImGui::ClearActiveID(); + auto zeroVec = [](ImVector& v) { + if (v.Data && v.Size > 0) sodium_memzero(v.Data, static_cast(v.Size)); + }; + zeroVec(st.TextA); // the live edit buffer + zeroVec(st.TextToRevertTo); // Escape-to-revert backup + zeroVec(st.CallbackTextBackup); // callback reconcile scratch + st.TextLen = 0; + // NOTE: st.Stb (the stb_textedit undo/redo char buffers) is deliberately NOT zeroed here — its type + // ImStbTexteditState is incomplete in imgui_internal.h (fully defined only inside ImGui's .cpp), so we + // can't sizeof it from a header. The primary copies above cover the committed text; the undo scratch + // is a residual gap in this best-effort scrub. + zeroVec(ctx->InputTextDeactivatedState.TextA); // the just-deactivated copy + ctx->InputTextDeactivatedState.ID = 0; +} + +} // namespace ui +} // namespace dragonx diff --git a/src/ui/windows/chat_tab.cpp b/src/ui/windows/chat_tab.cpp index 678a8e4..6235e41 100644 --- a/src/ui/windows/chat_tab.cpp +++ b/src/ui/windows/chat_tab.cpp @@ -2484,11 +2484,16 @@ void ResetChatTab() s_show_blocked = false; // Drop the per-frame memoization caches so the next wallet doesn't briefly render the previous one's // conversations/thread (store.revision() is monotonic and would rebuild anyway, but be explicit). + // Zero the decrypted plaintext the memo caches hold (conversation previews + full thread bodies) + // before dropping them, so the previous wallet's messages don't linger in freed heap. + auto wipeStr2 = [](std::string& s) { if (!s.empty()) sodium_memzero(&s[0], s.size()); s.clear(); }; + for (auto& c : s_convs) wipeStr2(c.lastBody); s_convs.clear(); s_convsHidden = 0; s_convsKey = ~0ull; s_threadCid.clear(); s_threadRev = ~0ull; + for (auto& m : s_threadMsgs) { wipeStr2(m.body); wipeStr2(m.fail_reason); wipeStr2(m.peer_zaddr); } s_threadMsgs.clear(); s_threadFlags.clear(); s_threadRows.clear(); s_threadHeights.clear(); s_threadPrefix.assign(1, 0.0f); s_threadFailed.clear(); s_threadBodySizes.clear(); diff --git a/src/ui/windows/seed_display.h b/src/ui/windows/seed_display.h index c3750d1..d4a23f7 100644 --- a/src/ui/windows/seed_display.h +++ b/src/ui/windows/seed_display.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "imgui.h" #include "../layout.h" // Layout::dpiScale() @@ -15,19 +16,31 @@ namespace dragonx { namespace ui { +// Best-effort zero of a seed-word vector (each word holds part of the phrase). Call after rendering. +inline void WipeSeedWords(std::vector& words) +{ + for (auto& w : words) { if (!w.empty()) sodium_memzero(&w[0], w.size()); w.clear(); } + words.clear(); +} + // Split a whitespace-separated seed phrase into its individual words. inline std::vector SplitSeedWords(const std::string& phrase) { std::vector words; + words.reserve(32); // avoid reallocation stranding un-wiped moved-from copies in freed heap std::string cur; for (char c : phrase) { if (c == ' ' || c == '\t' || c == '\n' || c == '\r') { - if (!cur.empty()) { words.push_back(cur); cur.clear(); } + if (!cur.empty()) { + words.push_back(cur); + sodium_memzero(&cur[0], cur.size()); // zero the scratch copy before reuse + cur.clear(); + } } else { cur.push_back(c); } } - if (!cur.empty()) words.push_back(cur); + if (!cur.empty()) { words.push_back(cur); sodium_memzero(&cur[0], cur.size()); cur.clear(); } return words; } @@ -45,6 +58,7 @@ inline void RenderSeedWordGrid(const std::vector& words, float colS std::snprintf(cell, sizeof(cell), "%2zu. %s", i + 1, words[i].c_str()); const float need = ImGui::CalcTextSize(cell).x + 20.0f * dp; // cell width + inter-column gap if (need > colStride) colStride = need; + sodium_memzero(cell, sizeof(cell)); // the cell held a seed word } for (size_t i = 0; i < words.size(); ++i) { char cell[96]; @@ -52,6 +66,7 @@ inline void RenderSeedWordGrid(const std::vector& words, float colS ImGui::TextUnformatted(cell); if ((i % 4) != 3 && i + 1 < words.size()) ImGui::SameLine(((i % 4) + 1) * colStride); + sodium_memzero(cell, sizeof(cell)); } } diff --git a/src/wallet/lite_wallet_controller.cpp b/src/wallet/lite_wallet_controller.cpp index d86083d..bf4db1f 100644 --- a/src/wallet/lite_wallet_controller.cpp +++ b/src/wallet/lite_wallet_controller.cpp @@ -1235,6 +1235,14 @@ LiteImportResult LiteWalletController::importKey(std::string spendingOrViewingKe return importKeyCore(*bridge_, spendingOrViewingKey); // wipes the key } +// Recursively zero every string node of a parsed secret JSON tree (seed / private keys) so the parse +// copies don't linger in freed heap. Best-effort (nlohmann's own lexer scratch isn't reachable). +static void wipeJsonStrings(nlohmann::json& j) +{ + if (j.is_string()) { secureWipeLiteSecret(j.get_ref()); } + else if (j.is_object() || j.is_array()) { for (auto& el : j) wipeJsonStrings(el); } +} + LiteExportResult LiteWalletController::exportPrivateKeys(const std::string& optionalAddress) { LiteExportResult out; @@ -1243,13 +1251,18 @@ LiteExportResult LiteWalletController::exportPrivateKeys(const std::string& opti return out; } // Empty address -> export keys for all addresses; otherwise just the given address. - const auto result = bridge_->execute("export", optionalAddress); + auto result = bridge_->execute("export", optionalAddress); // non-const: result.value is a secret if (!result.ok) { out.error = result.error.empty() ? "export failed" : result.error; return out; } + nlohmann::json j; + // result.value + the parse tree hold the private keys; wipe them on every return path. out already + // has its own copy (out.privateKeysJson) which the caller wipes after use. + struct Scrub { std::string& v; nlohmann::json& tree; ~Scrub() { secureWipeLiteSecret(v); wipeJsonStrings(tree); } } + scrub{result.value, j}; try { - const auto j = nlohmann::json::parse(result.value); + j = nlohmann::json::parse(result.value); if (j.is_object() && j.contains("error")) { out.error = extractJsonError(j); return out; @@ -1270,13 +1283,18 @@ LiteSeedResult LiteWalletController::exportSeed() out.error = "no wallet is open"; return out; } - const auto result = bridge_->execute("seed", ""); + auto result = bridge_->execute("seed", ""); // non-const: result.value holds the seed if (!result.ok) { out.error = result.error.empty() ? "seed export failed" : result.error; return out; } + nlohmann::json j; + // result.value + the parse tree contain the seed phrase; wipe them on every return path (out.seedPhrase + // is extracted separately and returned to the caller). + struct Scrub { std::string& v; nlohmann::json& tree; ~Scrub() { secureWipeLiteSecret(v); wipeJsonStrings(tree); } } + scrub{result.value, j}; try { - const auto j = nlohmann::json::parse(result.value); + j = nlohmann::json::parse(result.value); if (j.is_object()) { if (j.contains("error")) { out.error = extractJsonError(j);