From 69f59af54d15ee9800d5df86bcdb0e962c71e7c3 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 13 Jul 2020 03:01:57 +0000 Subject: [PATCH 1/2] Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks --- src/wallet/walletutil.cpp | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 2f3e597b90..6563c45134 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -49,26 +49,31 @@ std::vector ListWalletDir() continue; } - // Get wallet path relative to walletdir by removing walletdir from the wallet path. - // This can be replaced by boost::filesystem::lexically_relative once boost is bumped to 1.60. - const fs::path path = it->path().string().substr(offset); + try { + // Get wallet path relative to walletdir by removing walletdir from the wallet path. + // This can be replaced by boost::filesystem::lexically_relative once boost is bumped to 1.60. + const fs::path path = it->path().string().substr(offset); - if (it->status().type() == fs::directory_file && - (ExistsBerkeleyDatabase(it->path()) || ExistsSQLiteDatabase(it->path()))) { - // Found a directory which contains wallet.dat btree file, add it as a wallet. - paths.emplace_back(path); - } else if (it.level() == 0 && it->symlink_status().type() == fs::regular_file && ExistsBerkeleyDatabase(it->path())) { - if (it->path().filename() == "wallet.dat") { - // Found top-level wallet.dat btree file, add top level directory "" - // as a wallet. - paths.emplace_back(); - } else { - // Found top-level btree file not called wallet.dat. Current bitcoin - // software will never create these files but will allow them to be - // opened in a shared database environment for backwards compatibility. - // Add it to the list of available wallets. + if (it->status().type() == fs::directory_file && + (ExistsBerkeleyDatabase(it->path()) || ExistsSQLiteDatabase(it->path()))) { + // Found a directory which contains wallet.dat btree file, add it as a wallet. paths.emplace_back(path); + } else if (it.level() == 0 && it->symlink_status().type() == fs::regular_file && ExistsBerkeleyDatabase(it->path())) { + if (it->path().filename() == "wallet.dat") { + // Found top-level wallet.dat btree file, add top level directory "" + // as a wallet. + paths.emplace_back(); + } else { + // Found top-level btree file not called wallet.dat. Current bitcoin + // software will never create these files but will allow them to be + // opened in a shared database environment for backwards compatibility. + // Add it to the list of available wallets. + paths.emplace_back(path); + } } + } catch (const std::exception& e) { + LogPrintf("%s: Error scanning %s: %s\n", __func__, it->path().string(), e.what()); + it.no_push(); } } From 24d2d3341d07509ad3f37bb6f130446ad20ac807 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 13 Jul 2020 03:02:26 +0000 Subject: [PATCH 2/2] QA: wallet_multiwallet: Check that recursive symlink directory and wallet.dat loops are ignored --- test/functional/wallet_multiwallet.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index abdc279197..cf55b28afb 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -10,6 +10,7 @@ from threading import Thread import os import shutil +import stat import time from test_framework.authproxy import JSONRPCException @@ -78,6 +79,11 @@ def wallet_file(name): os.mkdir(wallet_dir('w7')) os.symlink('w7', wallet_dir('w7_symlink')) + os.symlink('..', wallet_dir('recursive_dir_symlink')) + + os.mkdir(wallet_dir('self_walletdat_symlink')) + os.symlink('wallet.dat', wallet_dir('self_walletdat_symlink/wallet.dat')) + # rename wallet.dat to make sure plain wallet file paths (as opposed to # directory paths) can be loaded # create another dummy wallet for use in testing backups later @@ -117,7 +123,16 @@ def wallet_file(name): self.nodes[0].createwallet(wallet_name) for wallet_name in to_load: self.nodes[0].loadwallet(wallet_name) - assert_equal(sorted(map(lambda w: w['name'], self.nodes[0].listwalletdir()['wallets'])), sorted(in_wallet_dir)) + + os.mkdir(wallet_dir('no_access')) + os.chmod(wallet_dir('no_access'), 0) + try: + with self.nodes[0].assert_debug_log(expected_msgs=['Too many levels of symbolic links', 'Error scanning']): + walletlist = self.nodes[0].listwalletdir()['wallets'] + finally: + # Need to ensure access is restored for cleanup + os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR) + assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir)) assert_equal(set(node.listwallets()), set(wallet_names))