fix: validate Electrum import input boundaries

src/wallet/rpc/backup.cpp

@@ -651,7 +651,7 @@ RPCHelpMan importelectrumwallet()
         "Note: This command is only compatible with legacy wallets.\n",
         {
             {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The Electrum wallet export file, should be in csv or json format"},
-            {"index", RPCArg::Type::NUM, RPCArg::Default{0}, "Rescan the wallet for transactions starting from this block index"},
+            {"index", RPCArg::Type::NUM, RPCArg::Default{0}, "Rescan the wallet for transactions starting from this non-negative block index"},
         },
         RPCResult{RPCResult::Type::NONE, "", ""},
         RPCExamples{
@@ -681,6 +681,11 @@ RPCHelpMan importelectrumwallet()
 
     EnsureWalletIsUnlocked(*pwallet);
 
+    int nStartHeight = request.params[1].isNull() ? 0 : request.params[1].getInt<int>();
+    if (nStartHeight < 0) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Rescan index must be non-negative");
+    }
+
     std::ifstream file;
     std::string strFileName = request.params[0].get_str();
     size_t nDotPos = strFileName.find_last_of('.');
@@ -691,15 +696,19 @@ RPCHelpMan importelectrumwallet()
     if(strFileExt != "json" && strFileExt != "csv")
         throw JSONRPCError(RPC_INVALID_PARAMETER, "File has wrong extension, should be .json or .csv");
 
-    file.open(strFileName, std::ios::in | std::ios::ate);
+    file.open(strFileName, std::ios::in | std::ios::ate | std::ios::binary);
     if (!file.is_open())
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open Electrum wallet export file");
 
     bool fGood = true;
 
     WalletBatch batch(pwallet->GetDatabase());
 
-    int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg());
+    const auto file_size = file.tellg();
+    if (file_size < 0) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot read Electrum wallet export file");
+    }
+    const int64_t nFilesize = std::max<int64_t>(1, file_size);
     file.seekg(0, file.beg);
 
     pwallet->ShowProgress(_("Importing…").translated, 0); // show progress dialog in GUI
@@ -750,9 +759,13 @@ RPCHelpMan importelectrumwallet()
         // json
         UniValue data(UniValue::VOBJ);
         {
-            auto buffer = std::make_unique<char[]>(nFilesize);
-            file.read(buffer.get(), nFilesize);
-            if(!data.read(buffer.get()))
+            std::string buffer(nFilesize, '\0');
+            file.read(buffer.data(), buffer.size());
+            if (file.bad() || (file.fail() && !file.eof())) {
+                throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot read Electrum wallet export file");
+            }
+            buffer.resize(file.gcount());
+            if (!data.read(buffer))
                 throw JSONRPCError(RPC_TYPE_ERROR, "Cannot parse Electrum wallet export file");
         }
 
@@ -798,9 +811,6 @@ RPCHelpMan importelectrumwallet()
     const int32_t tip_height = pwallet->chain().getHeight().value_or(std::numeric_limits<int32_t>::max());
 
     // Whether to perform rescan after import
-    int nStartHeight = 0;
-    if (!request.params[1].isNull())
-        nStartHeight = request.params[1].getInt<int>();
     if (tip_height < nStartHeight)
         nStartHeight = tip_height;
 

test/functional/wallet_dump.py

@@ -4,6 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the dumpwallet RPC."""
 import datetime
+import json
 import os
 import time
 
@@ -100,7 +101,39 @@ def setup_network(self):
         self.add_nodes(self.num_nodes, extra_args=self.extra_args)
         self.start_nodes()
 
+    def test_electrum_import(self):
+        node = self.nodes[0]
+        node.createwallet('electrum_source', descriptors=False)
+        node.createwallet('electrum_destination', blank=True, descriptors=False)
+        source = node.get_wallet_rpc('electrum_source')
+        destination = node.get_wallet_rpc('electrum_destination')
+        addresses = [source.getnewaddress() for _ in range(4)]
+        secrets = [source.dumpprivkey(address) for address in addresses]
+        json_file = os.path.join(node.datadir, 'electrum.json')
+        csv_file = os.path.join(node.datadir, 'electrum.csv')
+        with open(json_file, 'wb') as f:
+            f.write(json.dumps({addresses[0]: secrets[0]}).encode() + b'\x00trailing bytes')
+        assert_raises_rpc_error(-3, 'Cannot parse Electrum wallet export file', destination.importelectrumwallet, json_file)
+        assert not destination.getaddressinfo(addresses[0])['ismine']
+        with open(json_file, 'w', encoding='utf8') as f:
+            json.dump({addresses[0]: secrets[0], addresses[1]: 'p2pkh:' + secrets[1]}, f)
+        with open(csv_file, 'w', encoding='utf8') as f:
+            f.write('address,private_key\n' + addresses[2] + ',' + secrets[2] + '\n' + addresses[3] + ',p2pkh:' + secrets[3] + '\n')
+        for filename, import_addresses in ((json_file, addresses[:2]), (csv_file, addresses[2:])):
+            assert_raises_rpc_error(-8, 'Rescan index must be non-negative', destination.importelectrumwallet, filename, -1)
+            assert all(not destination.getaddressinfo(address)['ismine'] for address in import_addresses)
+        destination.importelectrumwallet(json_file, 0)
+        destination.importelectrumwallet(csv_file, node.getblockcount())
+        destination.importelectrumwallet(json_file, node.getblockcount() + 100)
+        for address, secret in zip(addresses, secrets):
+            assert_equal(destination.dumpprivkey(address), secret)
+            signature = destination.signmessage(address, 'Electrum import regression')
+            assert node.verifymessage(address, signature, 'Electrum import regression')
+        source.unloadwallet()
+        destination.unloadwallet()
+
     def run_test(self):
+        self.test_electrum_import()
         self.nodes[0].createwallet("dump")
 
         wallet_unenc_dump = os.path.join(self.nodes[0].datadir, "wallet.unencrypted.dump")