merge bitcoin#24026: Block unsafe std::string fs::path conversion copy_file calls

This commit is contained in:
Kittywhiskers Van Gogh 2024-08-06 17:40:25 +00:00
parent b0d2484a0b
commit 7c270e6883
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
6 changed files with 16 additions and 8 deletions

View File

@ -92,6 +92,13 @@ static inline path operator+(path p1, path p2)
return p1; return p1;
} }
// Disallow implicit std::string conversion for copy_file
// to avoid locale-dependent encoding on Windows.
static inline void copy_file(const path& from, const path& to, copy_option options)
{
boost::filesystem::copy_file(from, to, options);
}
/** /**
* Convert path object to byte string. On POSIX, paths natively are byte * Convert path object to byte string. On POSIX, paths natively are byte
* strings, so this is trivial. On Windows, paths natively are Unicode, so an * strings, so this is trivial. On Windows, paths natively are Unicode, so an

View File

@ -6,7 +6,7 @@
#define BITCOIN_INTERFACES_WALLET_H #define BITCOIN_INTERFACES_WALLET_H
#include <amount.h> // For CAmount #include <amount.h> // For CAmount
#include <fs.h> // For fs::path #include <fs.h>
#include <interfaces/chain.h> // For ChainClient #include <interfaces/chain.h> // For ChainClient
#include <pubkey.h> // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation) #include <pubkey.h> // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation)
#include <script/standard.h> // For CTxDestination #include <script/standard.h> // For CTxDestination
@ -347,7 +347,7 @@ public:
virtual std::string getWalletDir() = 0; virtual std::string getWalletDir() = 0;
//! Restore backup wallet //! Restore backup wallet
virtual std::unique_ptr<Wallet> restoreWallet(const std::string& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; virtual std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
//! Return available wallets in wallet directory. //! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0; virtual std::vector<std::string> listWalletDir() = 0;

View File

@ -607,7 +607,7 @@ public:
assert(m_context.m_coinjoin_loader); assert(m_context.m_coinjoin_loader);
return MakeWallet(LoadWallet(*m_context.chain, *m_context.m_coinjoin_loader, name, true /* load_on_start */, options, status, error, warnings)); return MakeWallet(LoadWallet(*m_context.chain, *m_context.m_coinjoin_loader, name, true /* load_on_start */, options, status, error, warnings));
} }
std::unique_ptr<Wallet> restoreWallet(const std::string& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) override std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
{ {
DatabaseStatus status; DatabaseStatus status;
assert(m_context.m_coinjoin_loader); assert(m_context.m_coinjoin_loader);

View File

@ -3084,7 +3084,7 @@ static RPCHelpMan restorewallet()
bilingual_str error; bilingual_str error;
std::vector<bilingual_str> warnings; std::vector<bilingual_str> warnings;
const std::shared_ptr<CWallet> wallet = RestoreWallet(*context.chain, *context.m_coinjoin_loader, fs::PathToString(backup_file), wallet_name, load_on_start, status, error, warnings); const std::shared_ptr<CWallet> wallet = RestoreWallet(*context.chain, *context.m_coinjoin_loader, backup_file, wallet_name, load_on_start, status, error, warnings);
HandleWalletError(wallet, status, error); HandleWalletError(wallet, status, error);

View File

@ -365,12 +365,12 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, interfaces::Coin
return wallet; return wallet;
} }
std::shared_ptr<CWallet> RestoreWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) std::shared_ptr<CWallet> RestoreWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, 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)
{ {
DatabaseOptions options; DatabaseOptions options;
options.require_existing = true; options.require_existing = true;
if (!fs::exists(fs::u8path(backup_file))) { if (!fs::exists(backup_file)) {
error = Untranslated("Backup file does not exist"); error = Untranslated("Backup file does not exist");
status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE; status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE;
return nullptr; return nullptr;
@ -5232,7 +5232,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error
} }
if(fs::exists(sourceFile)) { if(fs::exists(sourceFile)) {
try { try {
fs::copy_file(sourceFile, backupFile); fs::copy_file(sourceFile, backupFile, fs::copy_option::fail_if_exists);
WalletLogPrintf("Creating backup of %s -> %s\n", fs::PathToString(sourceFile), fs::PathToString(backupFile)); WalletLogPrintf("Creating backup of %s -> %s\n", fs::PathToString(sourceFile), fs::PathToString(backupFile));
} catch(fs::filesystem_error &error) { } catch(fs::filesystem_error &error) {
warnings.push_back(strprintf(_("Failed to create backup, error: %s"), fsbridge::get_filesystem_error_message(error))); warnings.push_back(strprintf(_("Failed to create backup, error: %s"), fsbridge::get_filesystem_error_message(error)));

View File

@ -8,6 +8,7 @@
#define BITCOIN_WALLET_WALLET_H #define BITCOIN_WALLET_WALLET_H
#include <amount.h> #include <amount.h>
#include <fs.h>
#include <governance/common.h> #include <governance/common.h>
#include <interfaces/chain.h> #include <interfaces/chain.h>
#include <interfaces/coinjoin.h> #include <interfaces/coinjoin.h>
@ -62,7 +63,7 @@ std::vector<std::shared_ptr<CWallet>> GetWallets();
std::shared_ptr<CWallet> GetWallet(const std::string& name); std::shared_ptr<CWallet> GetWallet(const std::string& name);
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, 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> LoadWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, 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(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, 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> CreateWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, 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(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, const std::string& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); std::shared_ptr<CWallet> RestoreWallet(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoin_loader, 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);
std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet); std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet);
std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);