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

fix: security-audit remediation (15 findings), empty-wallet warning, and send/chat/console/shutdown UX

Browse Source 
Security audit remediation (15 confirmed findings from the codebase audit):
- H-02: scrub+delete the decrypt-flow plaintext key export on ALL exit paths
  (RAII guard) and purge stale obsidiandecryptexport* files at startup.
- M-01/L-03/L-04/L-05/L-07: sodium_memzero the Set-PIN and encrypt-PIN worker
  passphrase/PIN copies, the RPC Basic-auth string (auth_), the exported/imported
  key buffers (App::wipeSecrets, called from ~App and before main's _Exit), and
  the first-run wizard "Skip" buffers.
- M-03/M-04/M-05/L-06: return locked COPIES from XmrigManager/EmbeddedDaemon
  getters (dedicated error_mutex_; DaemonController::lastError now by value),
  route xmrig last_error_ writes through a locked setter, and wrap
  shutdown_status_/wizard_stop_status_ in a locking GuardedStatus
  (wizard_stopping_external_ -> std::atomic).
- M-02: persist after a console send/shield/import in the lite backend.
- L-01: require the confirm click for z_shieldcoinbase/z_mergetoaddress.
- L-02: quote/escape each Windows daemon argv per the MSDN CommandLineToArgvW rules.
- L-08: pin json/tomlplusplus/libwebp FetchContent to immutable commit SHAs.
- I-01: extract updater archives from the already-verified in-memory buffer
  (no disk re-read TOCTOU).

Feature: warn once (full-node) when the active wallet loads empty while a sibling
wallet file in the datadir holds keys. A funded salvage wallet.<ts>.bak routes to
the recovery/Restore flow; a funded sibling .dat routes to the wallet manager.
Per-wallet-file dismissal; gated on synced + address-list-loaded to avoid false
positives on warm reconnect / spent-down wallets.

UX fixes:
- send: show the TOTAL balance (with a spendable "available" note) in the source
  dropdown and keep pending-change addresses visible.
- chat: insert emoji at the cursor position; restrict new-chat recipients to
  shielded (z) addresses.
- console: optional auto-focus of the command input on tab open (off by default).
- shutdown: when "stop external daemon" is on, keep the shutdown screen up until
  the external node actually exits, showing live status.

Adversarially reviewed; verified across full-node, lite, and Windows builds; tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-08-18 13:18:49 -05:00
parent ea26c0cbbb
commit 6ee81a5abe
40 changed files with 745 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

117
src/app_network.cpp
View File

@@ -620,6 +620,13 @@ void App::onConnected()
detectWalletAutoRecovery(); // also runs every tryConnect tick — catches a salvage even if we never connect
// Re-arm the empty-wallet-with-funded-sibling check for this (possibly switched) wallet: re-evaluate the
// on-disk state once it finishes loading + syncing. Deliberately DON'T touch empty_wallet_scan_in_flight_
// here — a scan from a prior connect self-clears it when it posts back, and resetting it while that scan
// is still running would let the next submit() block the UI thread on join() (async_tasks_ is only ever
// cancelled at shutdown, so the flag can't wedge during normal runtime).
empty_wallet_warn_checked_ = false;
// Stamp the active wallet as opened in the index (last-opened + size + synced-here). Balance +
// address count fill in on the first address refresh (addresses aren't loaded yet here).
updateWalletIndexForActiveWallet(/*markOpened=*/true);
@@ -1761,7 +1768,12 @@ void App::refreshAddressData()
auto result = NetworkRefreshService::collectAddressRefreshResult(refreshRpc, addressSnapshot);
return [this, previousAddressCount, previousWalletIdentity, result = std::move(result)]() mutable {
const bool addrListOk = result.addressListOk; // capture before the move
NetworkRefreshService::applyAddressRefreshResult(state_, std::move(result));
// Mark the address list as loaded ONLY if enumeration actually succeeded — a swallowed
// z_listaddresses/getaddressesbyaccount failure returns a falsely-short list, and stamping it
// would let the empty-wallet warning trust a spurious 0 count (see maybeWarnEmptyWallet…).
if (addrListOk) state_.last_address_update = std::time(nullptr);
applyPendingSendBalanceDeltas(false);
address_validation_cache_dirty_ = false;
address_list_dirty_ = true;
@@ -4190,6 +4202,111 @@ void App::maybeRemindSeedBackup()
});
}
// Complementary on-disk safety net (full-node) for a wallet salvage we did NOT witness this launch — it
// happened on a prior run, or under an external daemon whose startup output we never captured, so
// detectWalletAutoRecovery() never fired. If the active wallet loads EMPTY while a sibling wallet file in
// the datadir still holds keys, warn once: the user's funds were likely moved into a wallet.<ts>.bak by a
// prior BDB salvage and are not lost, just in another file. Fires at most once per wallet-open and once per
// unacknowledged wallet filename; the probe runs off the UI thread (scanFundedSiblingsAsync).
void App::maybeWarnEmptyWalletWithFundedSiblings()
{
if (capture_mode_) return; // no live ops during a UI sweep
if (lite_wallet_ || !supportsFullNodeLifecycleActions()) return; // full-node only (lite = single-file dir)
if (!settings_) return;
if (empty_wallet_warn_checked_ || empty_wallet_scan_in_flight_) return; // at most once per wallet-open
if (show_empty_wallet_warning_) return; // already surfaced
// The console-driven recovery flow owns the salvage-this-launch case — don't double-warn.
if (wallet_auto_recovered_ || show_wallet_recovered_dialog_) return;
// Only meaningful once the wallet is truly loaded AND fully synced: a mid-sync wallet reads empty.
if (!state_.connected || !state_.encryption_state_known) return;
if (state_.warming_up || state_.daemon_initializing || !state_.sync.isSynced()) return;
// Wait for the first Core refresh to land. The ConnectionInit prefetch sets sync.blocks but NOT headers,
// so isSynced() (blocks >= headers-2) is spuriously true in the window before the Core refresh — during
// which balance/addresses also read 0. last_balance_update flips non-zero only when the Core refresh
// applies (network_refresh_service.cpp), by which point balance & headers are real.
if (state_.last_balance_update == 0) return;
// And wait for the ADDRESS list to have loaded at least once — otherwise getAddressCount()==0 is
// ambiguous ("no keys" vs "not fetched yet"), which would false-fire on a spent-down wallet (0 balance
// but has addresses) whose address refresh lands a beat after the balance refresh.
if (state_.last_address_update == 0) return;
// "Empty" = no addresses and no funds. A salvage-created fresh wallet has no keys; a legitimately
// spent-down wallet keeps its addresses, so requiring zero addresses avoids nagging the latter.
if (state_.getAddressCount() != 0) return;
if (state_.totalBalance > 0.0 || state_.spendableTotalBalance > 0.0) return;
if (settings_->isEmptyWalletWarnAcked(settings_->getActiveWalletFile())) return; // dismissed for this file
empty_wallet_warn_checked_ = true; // evaluate the on-disk state once for this wallet-open
scanFundedSiblingsAsync();
}
// Off-UI-thread: enumerate the datadir's OTHER wallet files (incl. salvage wallet.<ts>.bak backups) and
// offline-probe each for key material. Read-only; never opens a file in the daemon. The probe reads up to a
// bounded prefix per file, so it runs on its own task thread (not the RPC worker) and the result is
// marshaled back to the main thread before touching any UI state. Routing, applied on the main thread:
// • a funded salvage .bak exists → the coins were moved aside by an unwitnessed salvage; hand off to the
// existing recovery dialog, whose Restore action swaps the .bak back (the correct, tested fix).
// • otherwise a funded sibling .dat exists → the user simply opened the wrong (empty) wallet; show the
// lightweight warning modal that routes to the wallet manager to switch.
void App::scanFundedSiblingsAsync()
{
if (empty_wallet_scan_in_flight_) return;
empty_wallet_scan_in_flight_ = true;
const std::string datadir = util::Platform::getDragonXDataDir();
const std::string activeFile = settings_ ? settings_->getActiveWalletFile() : std::string("wallet.dat");
async_tasks_.submit("Empty-wallet sibling scan",
[this, datadir, activeFile](const util::AsyncTaskManager::Token& tok) {
std::vector<FundedSibling> funded; // funded plain-.dat wallets → the "switch wallet" modal
bool hasSalvageBak = false; // a funded wallet.<ts>.bak → route to the recovery/restore dialog
for (const auto& path : util::enumerateDatadirWalletFiles(datadir, activeFile, /*includeSalvageBaks=*/true)) {
if (tok.cancelled()) return;
const auto bt = util::parseWalletBtree(path);
if (!(bt.parsed && bt.addresses() > 0)) continue; // ignore junk / empty siblings
const std::string name = std::filesystem::path(path).filename().string();
const bool isBak = name.size() > 4 && name.compare(name.size() - 4, 4, ".bak") == 0;
if (isBak) {
// Only a salvage-pattern wallet.<ts>.bak has a defined restore path; other .bak files are
// ignored (the wallet manager lists only .dat, so routing them there would be a dead end).
if (daemon::parseWalletSalvageBakTs(name) >= 0) hasSalvageBak = true;
continue;
}
FundedSibling s;
s.fileName = name;
s.transparentKeys = bt.transparentKeys;
s.shieldedKeys = bt.shieldedKeys;
funded.push_back(std::move(s));
}
// On teardown/cancel, skip posting (shutdown only; the in-flight flag is irrelevant then).
if (tok.cancelled() || !worker_) return;
// Apply UI state on the main thread only (the render loop reads these members).
worker_->post([this, funded, hasSalvageBak]() -> rpc::RPCWorker::MainCb {
return [this, funded, hasSalvageBak]() {
empty_wallet_scan_in_flight_ = false;
if (wallet_auto_recovered_ || show_wallet_recovered_dialog_) return; // recovery already owns it
// Re-validate emptiness on the main thread: balance/address refreshes may have landed while
// the scan ran (it takes long enough to read+parse sibling files), so a warm-reconnect or a
// spent-down wallet that momentarily read empty is now correctly excluded.
if (!state_.connected || state_.getAddressCount() != 0 ||
state_.totalBalance > 0.0 || state_.spendableTotalBalance > 0.0) return;
// Both cases surface OUR modal (renderEmptyWalletWarningDialog), keyed by has_salvage_bak.
// We deliberately DON'T set the wallet_auto_recovered_ latch or auto-open the recovery dialog:
// the daemon is healthy, and that latch gates the crash-restart loop (app_network.cpp:556) +
// crash-toast suppression, so it would wedge the wallet offline on any later unrelated crash.
// For the salvage case the modal's "Restore" button calls restoreOriginalWallet() directly
// (self-contained: it drives the recovery dialog into its Working phase itself).
if (hasSalvageBak) {
empty_wallet_has_salvage_bak_ = true;
empty_wallet_funded_siblings_.clear();
show_empty_wallet_warning_ = true;
} else if (!funded.empty()) {
empty_wallet_has_salvage_bak_ = false;
empty_wallet_funded_siblings_ = funded;
show_empty_wallet_warning_ = true;
}
};
});
});
}
// One-shot (per connect) probe of the current wallet's mnemonic status, so the Settings
// Migrate-to-seed button can glow for a legacy wallet without opening the migration dialog. Same
// classification as the migration Intro pre-flight, but proactive and cached. Reads no secret past