wallet: disallow unnamed wallets in createwallet and restorewallet
What changed, and why it matters
This Bitcoin Core change stops users from creating or restoring wallets with an empty name through the normal RPC commands. Empty wallet names could cause confusion or unexpected behavior, so the patch now rejects them with a clear error. The only exception is during wallet migration, where an existing unnamed wallet is allowed to be restored. The change is a hardening fix, not a response to an active exploit.
No urgent action required. Operators and integrators using createwallet/restorewallet should ensure they pass non-empty wallet names. The change is backward-incompatible for callers that previously relied on empty names, but that behavior is now considered unsupported except during migration.
Security signals we found
Input validation hardening: empty wallet name now rejected at API boundary
New error status code FAILED_NEW_UNNAMED introduced and mapped to RPC_INVALID_PARAMETER
Behavioral carve-out for migration only (allow_unnamed=true) to avoid breaking legacy migration
Functional tests updated to assert rejection of empty names
Evidence from the diff
The commit adds an explicit check in CreateWallet and RestoreWallet that rejects an empty wallet name, returning DatabaseStatus::FAILED_NEW_UNNAMED mapped to RPC_INVALID_PARAMETER (-8). A new allow_unnamed parameter is added to RestoreWallet, defaulting to false, and is only set true when MigrateLegacyToDescriptor restores a backup of a legacy unnamed wallet. Tests are updated to expect the new error and to create unnamed wallets indirectly only where needed for startup behavior testing.
Changed components
src/wallet/wallet.cpp (CreateWallet, RestoreWallet, MigrateLegacyToDescriptor)src/wallet/wallet.h (RestoreWallet signature)src/wallet/db.h (DatabaseStatus enum)src/wallet/rpc/util.cpp (HandleWalletError mapping)test/functional/wallet_backup.pytest/functional/wallet_createwallet.pytest/functional/wallet_startup.pyInspect captured patch +39 / −14
diff --git a/src/wallet/db.h b/src/wallet/db.h
index b953ab1f..b549e2a7 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -196,6 +196,7 @@ enum class DatabaseStatus {
FAILED_VERIFY,
FAILED_ENCRYPT,
FAILED_INVALID_BACKUP_FILE,
+ FAILED_NEW_UNNAMED,
};
/** Recursively list database paths in directory. */
diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp
index 46e7d437..012e0996 100644
--- a/src/wallet/rpc/util.cpp
+++ b/src/wallet/rpc/util.cpp
@@ -142,6 +142,7 @@ void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& st
case DatabaseStatus::FAILED_ALREADY_EXISTS:
code = RPC_WALLET_ALREADY_EXISTS;
break;
+ case DatabaseStatus::FAILED_NEW_UNNAMED:
case DatabaseStatus::FAILED_INVALID_BACKUP_FILE:
code = RPC_INVALID_PARAMETER;
break;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 56c676c1..f02b254c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -376,6 +376,13 @@ std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& n
std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings)
{
+ // Wallet must have a non-empty name
+ if (name.empty()) {
+ error = Untranslated("Wallet name cannot be empty");
+ status = DatabaseStatus::FAILED_NEW_UNNAMED;
+ return nullptr;
+ }
+
uint64_t wallet_creation_flags = options.create_flags;
const SecureString& passphrase = options.create_passphrase;
@@ -461,8 +468,16 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string&
// Re-creates wallet from the backup file by renaming and moving it into the wallet's directory.
// If 'load_after_restore=true', the wallet object will be fully initialized and appended to the context.
-std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings, bool load_after_restore)
+std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings, bool load_after_restore, bool allow_unnamed)
{
+ // Error if the wallet name is empty and allow_unnamed == false
+ // allow_unnamed == true is only used by migration to migrate an unnamed wallet
+ if (!allow_unnamed && wallet_name.empty()) {
+ error = Untranslated("Wallet name cannot be empty");
+ status = DatabaseStatus::FAILED_NEW_UNNAMED;
+ return nullptr;
+ }
+
DatabaseOptions options;
ReadDatabaseArgs(*context.args, options);
options.require_existing = true;
@@ -4442,7 +4457,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>
// Restore the backup
// Convert the backup file to the wallet db file by renaming it and moving it into the wallet's directory.
bilingual_str restore_error;
- const auto& ptr_wallet = RestoreWallet(context, backup_path, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/false);
+ const auto& ptr_wallet = RestoreWallet(context, backup_path, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/false, /*allow_unnamed=*/true);
if (!restore_error.empty()) {
error += restore_error + _("\nUnable to restore backup of wallet.");
return util::Error{error};
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 24d6b07d..b39827b6 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -97,7 +97,7 @@ std::shared_ptr<CWallet> GetDefaultWallet(WalletContext& context, size_t& count)
std::shared_ptr<CWallet> GetWallet(WalletContext& context, const std::string& name);
std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings);
std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings);
-std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings, bool load_after_restore = true);
+std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings, bool load_after_restore = true, bool allow_unnamed = false);
std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext& context, LoadWalletFn load_wallet);
void NotifyWalletLoaded(WalletContext& context, const std::shared_ptr<CWallet>& wallet);
std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 77d48fe5..b56cf64c 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -907,7 +907,6 @@ class TestNode():
def wait_until(self, test_function, timeout=60, check_interval=0.05):
return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.timeout_factor, check_interval=check_interval)
-
class TestNodeCLIAttr:
def __init__(self, cli, command):
self.cli = cli
diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py
index 5cf02ba3..06b8799d 100755
--- a/test/functional/wallet_backup.py
+++ b/test/functional/wallet_backup.py
@@ -184,13 +184,8 @@ class WalletBackupTest(BitcoinTestFramework):
# This is also useful to test the migration recovery after failure logic
node = self.nodes[3]
backup_file = self.nodes[0].datadir_path / 'wallet.bak'
- wallet_name = ""
- res = node.restorewallet(wallet_name, backup_file)
- assert_equal(res['name'], "")
- assert (node.wallets_path / "wallet.dat").exists()
- # Clean for follow-up tests
- node.unloadwallet("")
- os.remove(node.wallets_path / "wallet.dat")
+ assert_raises_rpc_error(-8, "Wallet name cannot be empty", node.restorewallet, "", backup_file)
+ assert not (node.wallets_path / "wallet.dat").exists()
def test_pruned_wallet_backup(self):
self.log.info("Test loading backup on a pruned node when the backup was created close to the prune height of the restoring node")
@@ -213,9 +208,8 @@ class WalletBackupTest(BitcoinTestFramework):
self.log.info("Test restore on a pruned node when the backup was beyond the pruning point")
backup_file = self.nodes[0].datadir_path / 'wallet.bak'
- wallet_name = ""
error_message = "Wallet loading failed. Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of a pruned node)"
- assert_raises_rpc_error(-4, error_message, node.restorewallet, wallet_name, backup_file)
+ assert_raises_rpc_error(-4, error_message, node.restorewallet, "restore_pruned", backup_file)
assert node.wallets_path.exists() # ensure the wallets dir exists
def run_test(self):
diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
index 9dd085bc..217a331d 100755
--- a/test/functional/wallet_createwallet.py
+++ b/test/functional/wallet_createwallet.py
@@ -32,6 +32,7 @@ class CreateWalletTest(BitcoinTestFramework):
# Run createwallet with invalid parameters. This must not prevent a new wallet with the same name from being created with the correct parameters.
assert_raises_rpc_error(-4, "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.",
self.nodes[0].createwallet, wallet_name='w0', disable_private_keys=True, passphrase="passphrase")
+ assert_raises_rpc_error(-8, "Wallet name cannot be empty", self.nodes[0].createwallet, "")
self.nodes[0].createwallet(wallet_name='w0')
w0 = node.get_wallet_rpc('w0')
diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py
index 17958b75..d2355360 100755
--- a/test/functional/wallet_startup.py
+++ b/test/functional/wallet_startup.py
@@ -6,6 +6,9 @@
Verify that a bitcoind node can maintain list of wallets loading on startup
"""
+import shutil
+import uuid
+
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
@@ -24,13 +27,24 @@ class WalletStartupTest(BitcoinTestFramework):
self.add_nodes(self.num_nodes)
self.start_nodes()
+ def create_unnamed_wallet(self, **kwargs):
+ """
+ createwallet disallows empty wallet names, so create a temporary named wallet
+ and move its wallet.dat to the unnamed wallet location
+ """
+ wallet_name = uuid.uuid4().hex
+ self.nodes[0].createwallet(wallet_name=wallet_name, **kwargs)
+ self.nodes[0].unloadwallet(wallet_name)
+ shutil.move(self.nodes[0].wallets_path / wallet_name / "wallet.dat", self.nodes[0].wallets_path / "wallet.dat")
+ shutil.rmtree(self.nodes[0].wallets_path / wallet_name)
+
def run_test(self):
self.log.info('Should start without any wallets')
assert_equal(self.nodes[0].listwallets(), [])
assert_equal(self.nodes[0].listwalletdir(), {'wallets': []})
self.log.info('New default wallet should load by default when there are no other wallets')
- self.nodes[0].createwallet(wallet_name='', load_on_startup=False)
+ self.create_unnamed_wallet(load_on_startup=False)
self.restart_node(0)
assert_equal(self.nodes[0].listwallets(), [''])
Why this scored 31/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.