The RPC console echoed and stored typed commands verbatim, so `walletpassphrase <secret>`, `z_importkey <key>`, `encryptwallet <pass>` etc. left the secret in the visible log AND the 100-entry recall history (copyable). Adds a pure, unit-testable RedactConsoleCommand()/ConsoleCommandCarriesSecret() (allowlist of 13 secret-bearing first-tokens) in console_tab_helpers; submitConsoleCommand() now echoes and stores `> walletpassphrase ****` while still executing the real command unredacted. Bare secret commands and non-secret commands pass through unchanged. Output-secret commands (dumpprivkey / z_exportkey / z_exportmnemonic) — whose secret is in the RESULT — are a separate redaction concern, tracked as a follow-up. First fix in the wallet-hardening P0-A cluster (see docs/wallet-hardening.md). New testConsoleSecretRedaction (11 assertions); ctest 1/1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7.4 KiB
7.4 KiB
Wallet Loading & Management — Hardening Plan
Prioritized, grouped remediation for the wallet loading/management audit (33 verified findings +
diagnosability QoL). Companion to the findings artifact. Line references are against dev.
- Provenance: 7 parallel subsystem finders, each finding adversarially verified against the code; the 3 highest-impact confirmed findings re-checked by hand. 32 confirmed, 1 refuted (W1-5), 1 raised (W5-3 Low→Med).
- Severity: 8 High · 12 Medium · 13 Low.
Status legend: ☐ not started · ◐ in progress · ☑ landed & verified
Roadmap (ordered by risk; shared fixes grouped)
| Phase | Findings | Theme | Status |
|---|---|---|---|
| P0-A | W7-1, W2-1, W4-1, W4-3, W2-3, W4-5, W5-3 | Secret hardening (SecureString + console redaction + delete-export) | ◐ |
| P0-B | W2-2/W4-2, W2-4 | Encryption integrity (never silently unencrypted) | ☐ |
| P1-A | W3-1, W3-2, W3-4, W3-3 | Migrate-to-seed correctness (fund-adjacent) | ☐ |
| P1-B | W1-1, W1-3, W1-2, W1-4 | Missing/wrong wallet-file safety | ☐ |
| P2 | W6-2, W5-1, W5-2, W6-1, W6-3 | Stale state & lite save-failure surfacing | ☐ |
| F | W7-2, W7-3, W7-4, QoL | Diagnostics foundation + QoL bundle | ☐ |
P0-A — Secret hardening
Shared fix: a SecureString RAII buffer (zeroes on destruction) retrofitted onto the un-scrubbed
key/passphrase paths, plus console redaction and deleting the plaintext export.
- W7-1 (High)
console_tab.cpp:1419— RPC console echoes/stores/clipboards raw secrets. Fix: an allowlist of secret-bearing first-tokens (walletpassphrase,walletpassphrasechange,encryptwallet,importprivkey,importwallet,z_importkey,z_importviewingkey,signrawtransaction,magicrecoverkey, lite equivalents); echo> walletpassphrase ****and keep the raw text out ofcommand_history_. Extract a pureredactConsoleCommand(cmd)helper for unit testing. ← implementing first (self-contained + testable). - W2-1 (High)
wallet_security_workflow.cpp:66— delete theobsidiandecryptexport<ts>plaintext key dump afterz_importwalletsucceeds (overwrite-then-unlink). - W4-3 (High)
app_network.cpp:4481—sodium_memzerothe concatenated all-keys string inexportAllKeys; write the backup 0600. (Also unify withExportAllKeysDialog— QoL.) - W4-1 (High)
app_network.cpp:3801— zero the key copies inimportPrivateKey/sweepPrivateKey(local + worker-lambda copies). - W2-3 (Med)
app_security.cpp:1481— zero the passphrase threaded through the decrypt lambda chain. - W4-5 (Med)
app.cpp:3577— the seed-backup.txtis a permanent predictable cleartext seed; at minimum warn + offer to delete, ideally discourage file save in favor of the on-screen phrase. - W5-3 (Med)
lite_wallet_lifecycle_service.cpp:322— remove the deadpassphrasefield from the lite create/open/restore requests (unused; a secret copied for nothing).
P0-B — Encryption integrity
- W2-2 / W4-2 (High)
wallet_security_controller.h:89— the wizard's deferred encryption is in-memory only and silently lost if the daemon doesn't connect or the app quits/crashes first, so a wallet the user believes is encrypted stays plaintext. Fix: persist a lightweightencryption_requested_but_incompletesettings flag (NEVER the passphrase) whenbeginDeferredEncryptionis called; surface a persistent warning banner while it's set; clear it only on confirmedencryptwalletsuccess; on next connect, if set, re-prompt for the passphrase to complete it. - W2-4 (Med)
app_security.cpp:480—lockWalletonly setslockedon RPC success; log the failure and notify (currently a silent no-op that can leave the wallet unlocked).
P1-A — Migrate-to-seed correctness (fund-adjacent; verify carefully)
- W3-1 (High)
app_network.cpp:4327— adopt hardcodesdatadir + "/wallet.dat"; usesettings_->getActiveWalletFile()so migrating a non-default active wallet swaps the right file. - W3-2 (High)
seed_wallet_creator.cpp:57—remove_all(<config>/seed-migrate)unconditionally at Phase-1 start; refuse to wipe if a tempDRAGONX/wallet.datalready exists (a prior un-adopted swept wallet) and surface it, so swept funds in the temp wallet can't be destroyed by re-entry. - W3-4 (Med)
app_network.cpp:1124— block wallet switching while a migration is pending (getSeedMigrationPending()), not only while the dialog is open. - W3-3 (Med)
app_network.cpp:4231— persist the sweep opid so an app-close mid-Sweeping can resume/re-poll it instead of silently dropping the txid.
P1-B — Missing/wrong wallet-file safety
- W1-1 (High)
app_network.cpp:1109—fs::exists()-check the target wallet file inswitchToWallet()and before the first daemon launch at startup; if missing, block with an explicit "Wallet file not found — moved or deleted?" dialog (browse / create-new) instead of letting the daemon fabricate an empty wallet. - W1-3 (Med)
app_network.cpp:1095— defer thesyncedHere=truestamp to the first successful address/balance readback (idHash non-empty), not bareonConnected(). - W1-2 (Med)
app_network.cpp:198— splitDB_CORRUPT-specific strings from the generic "Error loading wallet" fallback; giveDB_TOO_NEWits own message/action (not a salvage offer). - W1-4 (Low)
wallets_dialog.h:393— re-fs::exists()the in-datadir row before switching (match the out-of-datadir path).
P2 — State & lite persistence
- W6-2 (Med)
network_refresh_service.cpp:1183— record a per-field last-success timestamp / a "refresh failed" flag so the UI can show a staleness badge instead of last-good-as-current. - W5-1 / W5-2 (Med)
lite_wallet_controller.cpp:78,603—liteLog()the failed save and bubble a one-shot UI warning (both call sites currently discard the bool). - W6-1 (Med)
wallet_state.h:313— resetmining/pool_mininginclear()(or comment why not). - W6-3 (Low)
address_book.cpp:46— per-entry try/catch: skip + count malformed entries instead of discarding the whole list.
F — Diagnostics foundation + QoL
Land W7-2 first — it unblocks the rest.
- W7-2 (Med)
logger.cpp:31— callLogger::instance().init(<config>/dragonx-debug.log)early inmain()on all platforms; add an "Open log folder" action. - W7-3 (Med)
main.cpp:144— add asigaction-based crash handler writingdragonx-crash.logon POSIX (mirror the Windows SEH path). - W7-4 (Low)
logger.cpp:39— size-cap/rotate the log oninit(). - QoL — "Copy diagnostics for support" bundle; persistent alert history; daemon/RPC error banner; refresh-staleness badge; multi-wallet diagnostic panel; refresh-diagnostics panel; structured switch/migration audit logging; restore-from-seed entry point (W4-4, effort L).
Progress log
- P0-A / W7-1 — ☑ landed:
RedactConsoleCommand/ConsoleCommandCarriesSecretinconsole_tab_helpersredact secret-bearing commands (an allowlist of 13 first-tokens:walletpassphrase,encryptwallet,z_importkey, …) to> walletpassphrase ****before they hit the console echo AND the recall history; the real command still executes unredacted. Wired intosubmitConsoleCommand(console_tab.cpp). NewtestConsoleSecretRedaction(11 assertions). Clean build;ctest1/1. (Output-secret commands likez_exportkey— result redaction — remain a follow-up.)