From 7c270e68830562982045239a2b006a0503647a8a Mon Sep 17 00:00:00 2001
From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com>
Date: Tue, 6 Aug 2024 17:40:25 +0000
Subject: [PATCH] merge bitcoin#24026: Block unsafe std::string fs::path
 conversion copy_file calls

---
 src/fs.h                  | 7 +++++++
 src/interfaces/wallet.h   | 4 ++--
 src/wallet/interfaces.cpp | 2 +-
 src/wallet/rpcwallet.cpp  | 2 +-
 src/wallet/wallet.cpp     | 6 +++---
 src/wallet/wallet.h       | 3 ++-
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/fs.h b/src/fs.h
index 3cf4371fb4..80dd2c03d3 100644
--- a/src/fs.h
+++ b/src/fs.h
@@ -92,6 +92,13 @@ static inline path operator+(path p1, path p2)
     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
  * strings, so this is trivial. On Windows, paths natively are Unicode, so an
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index 32fb6b28e8..3bae75ed6f 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -6,7 +6,7 @@
 #define BITCOIN_INTERFACES_WALLET_H
 
 #include <amount.h>                    // For CAmount
-#include <fs.h>                        // For fs::path
+#include <fs.h>
 #include <interfaces/chain.h>          // For ChainClient
 #include <pubkey.h>                    // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation)
 #include <script/standard.h>           // For CTxDestination
@@ -347,7 +347,7 @@ public:
    virtual std::string getWalletDir() = 0;
 
    //! 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.
    virtual std::vector<std::string> listWalletDir() = 0;
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 6b93370bb2..b534c456a8 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -607,7 +607,7 @@ public:
         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));
     }
-    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;
         assert(m_context.m_coinjoin_loader);
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index d6718c64e9..e64428a375 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3084,7 +3084,7 @@ static RPCHelpMan restorewallet()
     bilingual_str error;
     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);
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 7e9dd44bb1..8cf5d8ba41 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -365,12 +365,12 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, interfaces::Coin
     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;
     options.require_existing = true;
 
-    if (!fs::exists(fs::u8path(backup_file))) {
+    if (!fs::exists(backup_file)) {
         error = Untranslated("Backup file does not exist");
         status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE;
         return nullptr;
@@ -5232,7 +5232,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error
         }
         if(fs::exists(sourceFile)) {
             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));
             } catch(fs::filesystem_error &error) {
                 warnings.push_back(strprintf(_("Failed to create backup, error: %s"), fsbridge::get_filesystem_error_message(error)));
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 35d00e4bae..4d399e57f2 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -8,6 +8,7 @@
 #define BITCOIN_WALLET_WALLET_H
 
 #include <amount.h>
+#include <fs.h>
 #include <governance/common.h>
 #include <interfaces/chain.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> 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> 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<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);