2 Commits

Author SHA1 Message Date
de1ae736de fix(wallet): guard against opening a missing/wrong wallet file (W1-1, W1-2, W1-4)
- W1-1 (High): switchToWallet never verified the target wallet file exists before
  switching. dragonxd auto-creates a fresh empty wallet for a missing -wallet=<name>, so a
  moved/deleted wallet file silently "opened" as a brand-new empty wallet with a zero
  balance — looking exactly like fund loss. It now std::filesystem::exists-checks
  datadir/<walletFile> before switching (ahead of the daemon-stop prompt) and blocks with a
  "not found (moved or deleted?)" warning. Because the check runs regardless of how
  switchToWallet is invoked, it also closes W1-4 (the stale switcher-row TOCTOU).

- W1-2 (Med): walletOutputLooksCorrupt matched the generic "Error loading wallet" string,
  which dragonxd also prints for DB_TOO_NEW (a newer-version wallet) — so a version mismatch
  was offered a -salvagewallet repair that cannot fix it. The generic match is now excluded
  when the output also contains "newer version".

Build-clean; ctest 1/1. Remaining P1-B: W1-3 (syncedHere timing) + the startup-path
existence check. See docs/wallet-hardening.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-02 14:56:59 -05:00
03c1b63e03 fix(migrate): correct fund-adjacent migrate-to-seed bugs (W3-1, W3-2, W3-4)
Migrate-to-seed (legacy -> mnemonic wallet) moves real funds; three correctness fixes:

- W3-1 (High): beginAdoptSeedWallet swapped a hardcoded datadir/wallet.dat instead of the
  ACTIVE wallet file. With a non-default active wallet (e.g. wallet-2.dat) it installed the
  swept seed wallet into an unloaded wallet.dat and left the daemon reloading the emptied
  legacy wallet — swept funds only recoverable via the seed phrase. Now swaps
  datadir + "/" + getActiveWalletFile(), captured on the main thread (switching is blocked
  during migration, so no race).

- W3-2 (High): SeedWalletCreator::create() ran remove_all(<config>/seed-migrate)
  unconditionally at the start, so a prior migration that swept funds into the temp wallet
  but was abandoned/crashed before adopting would have that fund-bearing wallet destroyed.
  It now refuses (with a clear message) when DRAGONX/wallet.dat already exists — a completed
  migration removes the dir on adopt, so a leftover means an unfinished one.

- W3-4 (Med): switchToWallet blocked switching only while the migration dialog was open;
  closing it via "Later" mid-migration dropped the guard. Now also blocks while
  getSeedMigrationPending().

Build-clean; ctest 1/1. Remaining P1-A: W3-3 (persist the sweep opid). See
docs/wallet-hardening.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-08-02 14:53:58 -05:00
3 changed files with 51 additions and 6 deletions

View File

@@ -18,8 +18,8 @@ Status legend: ☐ not started · ◐ in progress · ☑ landed & verified
|-------|----------|-------|--------|
| **P0-A** | W7-1, W2-1, W4-1, W4-3, W2-3, W4-5, W5-3 ✓ | Secret hardening (console redaction + delete-export + memzero + lite encrypt-at-create) | ☑ 7/7 |
| **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 | |
| **P1-A** | W3-1, W3-2, W3-4 ✓ · W3-3 | Migrate-to-seed correctness (fund-adjacent) | ◐ 3/4 |
| **P1-B** | W1-1, W1-2, W1-4 ✓ · W1-3 ☐ | Missing/wrong wallet-file safety | ◐ 3/4 |
| **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 | ☐ |
@@ -112,6 +112,16 @@ Land W7-2 first — it unblocks the rest.
## Progress log
- **P1-B / W1-1 (+ W1-4) · W1-2 (wallet-file safety)** — ☑ landed:
- **W1-1 (High):** `switchToWallet` never checked the target wallet file exists, so a moved/deleted file "opened" as a fresh empty wallet (dragonxd auto-creates for a missing `-wallet=`), looking exactly like fund loss. It now `std::filesystem::exists`-checks `datadir + "/" + walletFile` before switching and blocks with a "not found (moved or deleted?)" warning. Placed before the daemon-stop prompt, and — since the check runs no matter how `switchToWallet` is invoked — it also **closes W1-4** (the stale-switcher-row TOCTOU).
- **W1-2 (Med):** `walletOutputLooksCorrupt` matched the generic "Error loading wallet" string, so a `DB_TOO_NEW` (newer-version) wallet was offered a `-salvagewallet` repair that can't fix it. Now the generic match is excluded when the output also contains "newer version".
Build-clean; `ctest` 1/1. **Remaining P1-B:** W1-3 (defer the `syncedHere` stamp to a verified readback) + the startup-path existence check (`app.cpp` hands `getActiveWalletFile()` to the daemon with no `exists()` check — same silent-empty-wallet risk as W1-1 but at launch).
- **P1-A / W3-1 · W3-2 · W3-4 (migrate-to-seed correctness)** — ☑ landed (fund-adjacent — reviewed carefully):
- **W3-1 (High):** `beginAdoptSeedWallet` hardcoded `datadir + "/wallet.dat"` as the file to swap. With a non-default active wallet (e.g. `wallet-2.dat`), that installed the swept seed wallet into an unloaded `wallet.dat` and left the daemon reloading the emptied legacy — funds only recoverable via the seed phrase. Now swaps `datadir + "/" + getActiveWalletFile()` (captured on the main thread; switching is blocked during migration so it can't race).
- **W3-2 (High):** `SeedWalletCreator::create` did `remove_all(<config>/seed-migrate)` unconditionally at the start. A prior migration that swept funds into the temp wallet but was abandoned/crashed before adopting would have that fund-bearing wallet destroyed. It now refuses (with a clear message) when `DRAGONX/wallet.dat` already exists — a completed migration removes the dir on adopt, so a leftover means an unfinished one.
- **W3-4 (Med):** `switchToWallet` only blocked switching while the migration *dialog* was open; closing it via "Later" mid-migration dropped the guard. Now also blocks while `getSeedMigrationPending()`.
Build-clean; `ctest` 1/1. **Remaining P1-A:** W3-3 (persist the sweep opid so an app-close mid-sweep can resume/re-poll instead of silently dropping the txid).
- **P0-B / W2-2 (deferred encryption silently lost) + W2-4 (auto-lock silent-fail)** — ☑ landed:
- **W2-2:** the wizard's deferred encryption was stored only in memory, so a quit/crash or a failed daemon connect before it applied left the wallet unencrypted with **no record it was ever requested** — the user believing it was encrypted. Now a persisted `encryption_pending` settings flag is set the moment encryption is requested (**never the passphrase** — only the fact). `refreshWalletEncryptionState()` reconciles it on every connect: wallet observed **encrypted** → clear the flag; wallet **not** encrypted while the flag is set and no deferred encryption is pending/in-flight → a once-per-session **"your wallet is NOT encrypted — open Settings to finish"** warning (the flag stays set, so it recurs each launch until resolved). We deliberately don't persist the passphrase to auto-complete — surfacing it is the secure choice.
- **W2-4:** `lockWallet()`'s continuation only handled success — a failed `walletlock` silently left the wallet **unlocked** (an unfulfilled auto-lock). It now logs and warns once (reset on the next successful lock), so a failing auto-lock is visible instead of leaving the wallet exposed.

View File

@@ -197,10 +197,14 @@ static WarmupText translateWarmup(const std::string& raw)
// Used to offer a -salvagewallet repair when a switch fails because the target wallet is corrupt.
static bool walletOutputLooksCorrupt(const std::string& out)
{
// W1-2: the generic "Error loading wallet" fallback is ALSO printed for DB_TOO_NEW
// ("...requires ... newer version..."), which -salvagewallet cannot fix — so don't misclassify a
// version mismatch as salvageable corruption and offer a repair that can't help.
const bool versionMismatch = out.find("newer version") != std::string::npos;
return out.find("Failed to rename") != std::string::npos
|| out.find("salvage failed") != std::string::npos
|| out.find("wallet.dat corrupt") != std::string::npos
|| out.find("Error loading wallet") != std::string::npos;
|| (out.find("Error loading wallet") != std::string::npos && !versionMismatch);
}
// Phrases dragonxd prints to its console while initializing, in the order translateWarmup()
@@ -1121,7 +1125,9 @@ void App::switchToWallet(const std::string& walletFile, bool stopDaemonConfirmed
ui::Notifications::instance().warning("A rescan or repair is in progress — try again once it finishes.");
return;
}
if (show_seed_migration_) {
// W3-4: block switching while a migration is PENDING, not only while its dialog is open — closing
// the dialog via "Later" mid-migration leaves the pending state but previously dropped this guard.
if (show_seed_migration_ || (settings_ && settings_->getSeedMigrationPending())) {
ui::Notifications::instance().warning("Finish or cancel the seed migration before switching wallets.");
return;
}
@@ -1133,6 +1139,19 @@ void App::switchToWallet(const std::string& walletFile, bool stopDaemonConfirmed
ui::Notifications::instance().warning("Finish or cancel the pending send before switching wallets.");
return;
}
// W1-1: verify the target wallet file actually exists before switching. dragonxd auto-CREATES a
// fresh empty wallet for a missing -wallet=<name>, so without this a moved/deleted wallet file would
// silently "open" as a brand-new empty wallet with a zero balance — looking exactly like fund loss.
// (Also closes the W1-4 stale-switcher-row race: the check runs no matter how switchToWallet is called.)
{
std::error_code existEc;
const std::string walletPath = util::Platform::getDragonXDataDir() + "/" + walletFile;
if (!std::filesystem::exists(walletPath, existEc)) {
ui::Notifications::instance().warning(
"Wallet file not found (moved or deleted?): " + walletFile + " — it was not opened.", 15.0f);
return;
}
}
// If we're connected to a node this session did NOT spawn (no live process handle — it was left
// running by "keep node running", started by the user, or we just direct-connected to a config-provided
// one), confirm before stopping it: switching must stop+restart it on the new wallet, but the user may
@@ -4322,7 +4341,13 @@ void App::beginAdoptSeedWallet()
// has its own passphrase; the user can re-enable PIN quick-unlock for it).
if (vault_) vault_->removeVault();
const std::string base = seed_migration_temp_dir_;
async_tasks_.submit("Adopt seed wallet", [this, base](const util::AsyncTaskManager::Token&) {
// W3-1: adopt must swap the ACTIVE wallet file (multi-wallet), not a hardcoded "wallet.dat" —
// otherwise a migration run while e.g. wallet-2.dat is active would install the swept seed wallet
// into an unloaded wallet.dat and leave the daemon loading the (now-emptied) legacy wallet.
// Captured on the main thread; wallet switching is blocked during migration so this can't race.
const std::string activeWalletName = (settings_ && !settings_->getActiveWalletFile().empty())
? settings_->getActiveWalletFile() : std::string("wallet.dat");
async_tasks_.submit("Adopt seed wallet", [this, base, activeWalletName](const util::AsyncTaskManager::Token&) {
namespace fs = std::filesystem;
std::string err; // fatal (swap did not happen; migration incomplete)
std::string warn; // non-fatal (swap done but the daemon did not restart)
@@ -4342,7 +4367,7 @@ void App::beginAdoptSeedWallet()
// 2. Swap wallet.dat. Move the legacy one aside to a timestamped backup (NEVER
// delete), then copy the new seed wallet in. On any failure, restore the legacy.
const std::string datadir = util::Platform::getDragonXDataDir();
const std::string legacy = datadir + "/wallet.dat";
const std::string legacy = datadir + "/" + activeWalletName;
const std::string newWallet = base + "/DRAGONX/wallet.dat";
std::time_t t = std::time(nullptr);
std::tm tmv{}; // thread-safe local time (the UI thread also uses localtime)

View File

@@ -54,6 +54,16 @@ SeedWalletResult SeedWalletCreator::create(bool keepDatadir,
// RPC port. So the wallet lives in <base>/DRAGONX; `base` is the migration root we clean up.
const std::string base = util::Platform::getConfigDir() + "/seed-migrate";
const std::string dataDir = base + "/DRAGONX";
// W3-2: never blindly wipe a pre-existing temp seed wallet. A prior migration that swept funds into
// it but was abandoned or crashed before adopting would otherwise have its (fund-bearing) wallet
// destroyed here. A completed migration removes this dir on adopt, so a leftover means an unfinished
// one — refuse and point the user at it rather than silently destroying it.
if (fs::exists(dataDir + "/wallet.dat")) {
r.error = "A previous seed migration looks unfinished — its temporary wallet is still at\n" + base +
"\nResume or cancel it first. If you are certain its funds are already in your main "
"wallet, delete that folder and try again.";
return r;
}
fs::remove_all(base, ec);
fs::create_directories(dataDir, ec);
if (ec) { r.error = "Could not create the temporary wallet directory."; return r; }