Do not import private keys to wallets with private keys disabled

achow101 · achow101 · commit e6c58d3b014a · 2019-01-31T14:29:28.000-05:00
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
@@ -133,6 +133,9 @@ UniValue importprivkey(const JSONRPCRequest& request)
             + HelpExampleRpc("importprivkey", "\"mykey\", \"testing\", false")
         );
 
+    if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+        throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled");
+    }
 
     WalletRescanReserver reserver(pwallet);
     bool fRescan = true;
@@ -617,6 +620,11 @@ UniValue importwallet(const JSONRPCRequest& request)
             }
         }
         file.close();
+        // We now know whether we are importing private keys, so we can error if private keys are disabled
+        if (keys.size() > 0 && pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+            uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI
+            throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled when private keys are disabled");
+        }
         double total = (double)(keys.size() + scripts.size());
         double progress = 0;
         for (const auto& key_tuple : keys) {
@@ -967,6 +975,11 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con
         const bool watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false;
         const std::string& label = data.exists("label") ? data["label"].get_str() : "";
 
+        // If private keys are disabled, abort if private keys are being imported
+        if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !keys.isNull()) {
+            throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import private keys to a wallet with private keys disabled");
+        }
+
         // Generate the script and destination for the scriptPubKey provided
         CScript script;
         CTxDestination dest;
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -3830,6 +3830,10 @@ UniValue sethdseed(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download");
     }
 
+    if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+        throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed to a wallet with private keys disabled");
+    }
+
     auto locked_chain = pwallet->chain().lock();
     LOCK(pwallet->cs_wallet);
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -251,6 +251,9 @@ bool CWallet::AddKeyPubKeyWithDB(WalletBatch &batch, const CKey& secret, const C
 {
     AssertLockHeld(cs_wallet); // mapKeyMetadata
 
+    // Make sure we aren't adding private keys to private key disabled wallets
+    assert(!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
+
     // CCryptoKeyStore has no concept of wallet databases, but calls AddCryptedKey
     // which is overridden below.  To avoid flushes, the database handle is
     // tunneled through to it.
diff --git a/test/functional/wallet_disableprivatekeys.py b/test/functional/wallet_disableprivatekeys.py
@@ -7,6 +7,7 @@
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
+    assert_equal,
     assert_raises_rpc_error,
 )
 
@@ -31,5 +32,15 @@ def run_test(self):
         assert_raises_rpc_error(-4,"Error: Private keys are disabled for this wallet", w1.getrawchangeaddress)
         w1.importpubkey(w2.getaddressinfo(w2.getnewaddress())['pubkey'])
 
+        self.log.info('Test that private keys cannot be imported')
+        addr = w2.getnewaddress('', 'legacy')
+        privkey = w2.dumpprivkey(addr)
+        assert_raises_rpc_error(-4, 'Cannot import private keys to a wallet with private keys disabled', w1.importprivkey, privkey)
+        result = w1.importmulti([{'scriptPubKey': {'address': addr}, 'timestamp': 'now', 'keys': [privkey]}])
+        assert(not result[0]['success'])
+        assert('warning' not in result[0])
+        assert_equal(result[0]['error']['code'], -4)
+        assert_equal(result[0]['error']['message'], 'Cannot import private keys to a wallet with private keys disabled')
+
 if __name__ == '__main__':
     DisablePrivateKeysTest().main()

Original file line number	Diff line number	Diff line change
`@@ -3830,6 +3830,10 @@ UniValue sethdseed(const JSONRPCRequest& request)`
`3830`	`3830`	`throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download");`
`3831`	`3831`	`}`
`3832`	`3832`
	`3833`	`+ if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {`
	`3834`	`+ throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed to a wallet with private keys disabled");`
	`3835`	`+ }`
	`3836`	`+`
`3833`	`3837`	`auto locked_chain = pwallet->chain().lock();`
`3834`	`3838`	`LOCK(pwallet->cs_wallet);`
`3835`	`3839`
Original file line number	Diff line number	Diff line change
`@@ -251,6 +251,9 @@ bool CWallet::AddKeyPubKeyWithDB(WalletBatch &batch, const CKey& secret, const C`
`251`	`251`	`{`
`252`	`252`	`AssertLockHeld(cs_wallet); // mapKeyMetadata`
`253`	`253`
	`254`	`+ // Make sure we aren't adding private keys to private key disabled wallets`
	`255`	`+ assert(!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));`
	`256`	`+`
`254`	`257`	`// CCryptoKeyStore has no concept of wallet databases, but calls AddCryptedKey`
`255`	`258`	`// which is overridden below. To avoid flushes, the database handle is`
`256`	`259`	`// tunneled through to it.`