Merge #12101: Clamp walletpassphrase timeout to 2^30 seconds and check its bounds

134cdc7 Test walletpassphrase timeout bounds and clamping (Andrew Chow)
0b63e3c Clamp walletpassphrase timeout to 2^(30) seconds and check its bounds (Andrew Chow)

Pull request description:

  Fixes #12100

  Makes the timeout be clamped to 2^30 seconds to avoid the issue with sign flipping with large timeout values and thus relocking the wallet instantly. Unlocking for at most ~34 years should be sufficient.

  Also checks that the timeout is not negative to avoid instant relocks.

Tree-SHA512: 426922f08c54e323d259e25dcdbebc2cd560708a65111ce6051493a7e7c61e79d9da1ea4026cc0d68807d728f5d7c0d7c58168c6ef4167b94cf6c2877af88794

Author: laanwj

src/wallet/rpcwallet.cpp

@@ -2283,7 +2283,8 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
             "This is needed prior to performing transactions related to private keys such as sending bitcoins\n"
             "\nArguments:\n"
             "1. \"passphrase\"     (string, required) The wallet passphrase\n"
-            "2. timeout            (numeric, required) The time to keep the decryption key in seconds.\n"
+            "2. timeout            (numeric, required) The time to keep the decryption key in seconds. Limited to at most 1073741824 (2^30) seconds.\n"
+            "                                          Any value greater than 1073741824 seconds will be set to 1073741824 seconds.\n"
             "\nNote:\n"
             "Issuing the walletpassphrase command while the wallet is already unlocked will set a new unlock\n"
             "time that overrides the old one.\n"
@@ -2312,6 +2313,17 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
     // Alternately, find a way to make request.params[0] mlock()'d to begin with.
     strWalletPass = request.params[0].get_str().c_str();
 
+    // Get the timeout
+    int64_t nSleepTime = request.params[1].get_int64();
+    // Timeout cannot be negative, otherwise it will relock immediately
+    if (nSleepTime < 0) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative.");
+    }
+    // Clamp timeout to 2^30 seconds
+    if (nSleepTime > (int64_t)1 << 30) {
+        nSleepTime = (int64_t)1 << 30;
+    }
+
     if (strWalletPass.length() > 0)
     {
         if (!pwallet->Unlock(strWalletPass)) {
@@ -2325,7 +2337,6 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
 
     pwallet->TopUpKeyPool();
 
-    int64_t nSleepTime = request.params[1].get_int64();
     pwallet->nRelockTime = GetTime() + nSleepTime;
     RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), boost::bind(LockWallet, pwallet), nSleepTime);
 

test/functional/wallet-encryption.py

@@ -10,6 +10,8 @@
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
+    assert_greater_than,
+    assert_greater_than_or_equal,
 )
 
 class WalletEncryptionTest(BitcoinTestFramework):
@@ -56,6 +58,23 @@ def run_test(self):
         assert_raises_rpc_error(-14, "wallet passphrase entered was incorrect", self.nodes[0].walletpassphrase, passphrase, 10)
         self.nodes[0].walletpassphrase(passphrase2, 10)
         assert_equal(privkey, self.nodes[0].dumpprivkey(address))
+        self.nodes[0].walletlock()
+
+        # Test timeout bounds
+        assert_raises_rpc_error(-8, "Timeout cannot be negative.", self.nodes[0].walletpassphrase, passphrase2, -10)
+        # Check the timeout
+        # Check a time less than the limit
+        expected_time = int(time.time()) + (1 << 30) - 600
+        self.nodes[0].walletpassphrase(passphrase2, (1 << 30) - 600)
+        actual_time = self.nodes[0].getwalletinfo()['unlocked_until']
+        assert_greater_than_or_equal(actual_time, expected_time)
+        assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
+        # Check a time greater than the limit
+        expected_time = int(time.time()) + (1 << 30) - 1
+        self.nodes[0].walletpassphrase(passphrase2, (1 << 33))
+        actual_time = self.nodes[0].getwalletinfo()['unlocked_until']
+        assert_greater_than_or_equal(actual_time, expected_time)
+        assert_greater_than(expected_time + 5, actual_time) # 5 second buffer
 
 if __name__ == '__main__':
     WalletEncryptionTest().main()