security: scrub on-screen secrets on lock; redact lite console secret args; no-shell wallet rebuild
- Locking the wallet (manual / auto / lite) now calls wipeDisplayedSecrets(), which scrubs + closes any revealed seed phrase, migration seed, or exported key — previously a locked wallet could leave the phrase/key on screen and resident. Runs synchronously, before the async lock round-trip. - The console secret-arg redaction list only had full-node RPC names, so lite verbs (import / timport / encrypt / decrypt / unlock) echoed keys and passphrases into the scrollback and up-arrow history; added them. - The offline wallet-rebuild helper was invoked via a shell string (popen "sh -c") built from paths including the wallet filename, so a crafted name could inject a command; it now refuses any path with a shell metacharacter (rc stays -1 and the existing verify-before-swap fails safe). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+19
@@ -145,6 +145,25 @@ void App::wipeSecrets()
|
||||
sodium_memzero(import_clip_preview_, sizeof(import_clip_preview_)); // clipboard preview (SECRET)
|
||||
}
|
||||
|
||||
// Scrub + close any secret that is currently ON SCREEN (revealed seed phrase, migration seed, exported
|
||||
// key) when the wallet locks — locking shouldn't leave a phrase/key visible or resident. Called on every
|
||||
// lock transition (manual, auto, lite). Separate from wipeSecrets(), which is the exit-path scrub.
|
||||
void App::wipeDisplayedSecrets()
|
||||
{
|
||||
if (!seed_backup_phrase_.empty()) {
|
||||
sodium_memzero(&seed_backup_phrase_[0], seed_backup_phrase_.size());
|
||||
seed_backup_phrase_.clear();
|
||||
}
|
||||
if (!seed_migration_seed_.empty()) {
|
||||
sodium_memzero(&seed_migration_seed_[0], seed_migration_seed_.size());
|
||||
seed_migration_seed_.clear();
|
||||
}
|
||||
seed_backup_fetch_started_ = false;
|
||||
show_seed_backup_ = false;
|
||||
sodium_memzero(export_result_, sizeof(export_result_));
|
||||
ui::KeyExportDialog::hide(); // zeros its static s_key via secureWipeLiteSecret
|
||||
}
|
||||
|
||||
// Enqueue a blocking xmrig start/stop op onto the dedicated serialized control thread so the render thread
|
||||
// never blocks on stop()'s SIGTERM->SIGKILL->join, while start/stop still execute in FIFO order. (M-03/…)
|
||||
void App::postMiningControl(std::function<void()> job)
|
||||
|
||||
@@ -114,6 +114,7 @@ class App {
|
||||
public:
|
||||
App();
|
||||
void wipeSecrets(); // scrub all resident secret buffers; called from ~App() AND the forced-exit path (L-05)
|
||||
void wipeDisplayedSecrets(); // scrub + close any on-screen secret (seed/key displays) on wallet lock
|
||||
~App();
|
||||
|
||||
// Non-copyable
|
||||
|
||||
+15
-2
@@ -5399,6 +5399,7 @@ bool App::lockLiteWallet()
|
||||
if (!lite_wallet_) return false;
|
||||
const bool ok = lite_wallet_->lockWallet();
|
||||
if (ok) {
|
||||
wipeDisplayedSecrets(); // scrub any revealed seed/key display on lock (matches full-node)
|
||||
// Full-node App::lockWallet() sets state_.locked synchronously; the lite backend `lock`
|
||||
// doesn't, and state_.locked only refreshes on the next ~2s poll. Mirror the full-node
|
||||
// behavior here and tear down the chat session NOW so no decrypted store / unlocked DB key
|
||||
@@ -6917,9 +6918,21 @@ void App::rebuildWalletDatabase()
|
||||
// 3. Run the helper (src -> tmpOut). Quote both paths; capture its JSON line. Windowless
|
||||
// (runHiddenCapture) so a wallet rebuild never flashes a cmd.exe console; it runs the
|
||||
// helper via CreateProcess directly on Windows, so no cmd.exe outer-quote wrap is needed.
|
||||
const std::string cmd = "\"" + helper + "\" \"" + src + "\" \"" + tmpOut + "\"";
|
||||
// runHiddenCapture runs this via popen("sh -c …") on POSIX, where $( ), backticks and an
|
||||
// embedded \" still expand inside the double-quotes — so a crafted wallet filename (src is
|
||||
// datadir + the activeWalletName setting) could inject a shell command. Refuse any path
|
||||
// carrying a shell metacharacter; rc stays -1 and the verify-before-swap below fails safe.
|
||||
auto shellSafe = [](const std::string& p) {
|
||||
return p.find_first_of("\"'`$;&|<>^\n\r") == std::string::npos;
|
||||
};
|
||||
int rc = -1;
|
||||
const std::string jout = util::Platform::runHiddenCapture(cmd, /*mergeStderr=*/false, &rc);
|
||||
std::string jout;
|
||||
if (shellSafe(helper) && shellSafe(src) && shellSafe(tmpOut)) {
|
||||
const std::string cmd = "\"" + helper + "\" \"" + src + "\" \"" + tmpOut + "\"";
|
||||
jout = util::Platform::runHiddenCapture(cmd, /*mergeStderr=*/false, &rc);
|
||||
} else {
|
||||
DEBUG_LOGF("[App] wallet-rebuild refused: shell metacharacter in a path\n");
|
||||
}
|
||||
DEBUG_LOGF("[App] wallet-rebuild helper rc=%d out=%s\n", rc, jout.c_str());
|
||||
|
||||
// 4. Verify-before-swap: the output must be a readable BDB with the fund-critical keys.
|
||||
|
||||
@@ -598,6 +598,7 @@ void App::unlockWallet(const std::string& passphrase, int timeout) {
|
||||
void App::lockWallet() {
|
||||
if (!rpc_ || !rpc_->isConnected() || !worker_) return;
|
||||
if (lock_unlock_in_progress_) return; // Prevent duplicate async calls
|
||||
wipeDisplayedSecrets(); // hide any revealed seed/key immediately, before the async lock round-trip
|
||||
lock_unlock_in_progress_ = true;
|
||||
|
||||
// Use fast-lane worker to avoid blocking behind refreshData.
|
||||
|
||||
@@ -15,6 +15,10 @@ const char* const kSecretConsoleCommands[] = {
|
||||
"importprivkey", "importwallet", "importmulti",
|
||||
"z_importkey", "z_importviewingkey", "z_importwallet",
|
||||
"signrawtransaction", "magicrecoverkey", "sethdseed", "importmnemonic",
|
||||
// Lite-wallet verbs (console_command_reference kLiteKeyCommands): these take a spending/viewing key
|
||||
// or a password as their argument. Full-node has no bare import/encrypt/decrypt/unlock verb, so a
|
||||
// flat match is safe.
|
||||
"import", "timport", "encrypt", "decrypt", "unlock",
|
||||
};
|
||||
|
||||
std::string firstConsoleTokenLower(const std::string& cmd, size_t& tokenEnd) {
|
||||
|
||||
Reference in New Issue
Block a user