wallettool: Use GetWalletPath to determine the wallet path
What changed, and why it matters
This change refactors how the standalone bitcoin-wallet tool figures out where a wallet file lives. Previously it computed the path itself; now it calls the same internal helper (GetWalletPath) that the main wallet code uses, which includes checks for invalid wallet names. The likely effect is to prevent mistakes or misuse involving odd wallet names/paths, rather than to fix an active remote-exploitable bug.
Treat as a hardening/refactoring change. Review whether any prior release allowed wallet names that bypassed GetWalletPath checks in bitcoin-wallet; if so, consider a low-severity advisory. No urgent action required for typical deployments.
Security signals we found
Path validation centralized to a single trusted helper
Wallet tool previously computed path without the same checks as the main wallet
Rejects absolute wallet names and directory-traversal-style names
No explicit security framing by the commit author
Evidence from the diff
The patch makes GetWalletPath visible outside wallet.cpp and uses it in wallettool.cpp instead of a direct fsbridge::AbsPathJoin(GetWalletDir(), name). GetWalletPath rejects absolute paths and paths that walk outside the wallet directory (via fs::PathFromString and relative-path checks). This centralizes path validation for the wallet-tool CLI, closing a gap where the tool might have accepted a wallet name that the main node would reject or that could resolve to an unexpected filesystem location.
Changed components
src/wallet/wallettool.cppsrc/wallet/wallet.cppsrc/wallet/wallet.hbitcoin-wallet command-line toolInspect captured patch +10 / −2
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 49099bf7..a95d67e8 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2940,7 +2940,7 @@ bool CWallet::EraseAddressReceiveRequest(WalletBatch& batch, const CTxDestinatio
return true;
}
-static util::Result<fs::path> GetWalletPath(const std::string& name)
+util::Result<fs::path> GetWalletPath(const std::string& name)
{
const fs::path name_path = fs::PathFromString(name);
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 9cd78258..5385ee97 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -1150,6 +1150,9 @@ struct MigrationResult {
[[nodiscard]] util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context);
//! Requirement: The wallet provided to this function must be isolated, with no attachment to the node's context.
[[nodiscard]] util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> local_wallet, const SecureString& passphrase, WalletContext& context);
+
+//! Determine the path that the wallet is stored in
+util::Result<fs::path> GetWalletPath(const std::string& name);
} // namespace wallet
#endif // BITCOIN_WALLET_WALLET_H
diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp
index 7c24c0e1..d1f3a8ce 100644
--- a/src/wallet/wallettool.cpp
+++ b/src/wallet/wallettool.cpp
@@ -102,7 +102,12 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
return false;
}
const std::string name = args.GetArg("-wallet", "");
- const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(name));
+ util::Result<fs::path> path_res = GetWalletPath(name);
+ if (!path_res) {
+ tfm::format(std::cerr, "%s\n", util::ErrorString(path_res).original);
+ return false;
+ }
+ const fs::path& path = *path_res;
if (command == "create") {
if (name.empty()) {
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.