From de1ae736de8a642a54b258207faaeaf1c4007ac8 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 2 Aug 2026 14:56:59 -0500 Subject: [PATCH] fix(wallet): guard against opening a missing/wrong wallet file (W1-1, W1-2, W1-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - W1-1 (High): switchToWallet never verified the target wallet file exists before switching. dragonxd auto-creates a fresh empty wallet for a missing -wallet=, 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/ 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 --- docs/wallet-hardening.md | 6 +++++- src/app_network.cpp | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/wallet-hardening.md b/docs/wallet-hardening.md index 8738ec1..9715808 100644 --- a/docs/wallet-hardening.md +++ b/docs/wallet-hardening.md @@ -19,7 +19,7 @@ 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) | ◐ 3/4 | -| **P1-B** | W1-1, W1-3, W1-2, W1-4 | Missing/wrong wallet-file safety | ☐ | +| **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,10 @@ 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(/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. diff --git a/src/app_network.cpp b/src/app_network.cpp index e5d495b..dd89e7a 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -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() @@ -1135,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=, 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