Check script size as we decode it. Add test. Other minor fixes.

DeFiCh · Apr 7, 2021 · 6af59ba · 6af59ba
1 parent 1eb6574
commit 6af59ba
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 2 deletions.
diff --git a/src/spv/bitcoin/BRTransaction.cpp b/src/spv/bitcoin/BRTransaction.cpp
@@ -674,14 +674,15 @@ int BRTransactionSign(BRTransaction *tx, int forkId, BRKey keys[], size_t keysCo
     for (i = 0; tx && i < tx->inCount; i++) {
         BRTxInput *input = &tx->inputs[i];
         const uint8_t *hash;
+        UInt160 hash160 = UINT160_ZERO;
 
         if (htlcType == ScriptTypeNone)
         {
             hash = BRScriptPKH(input->script, input->scriptLen);
         }
         else
         {
-            UInt160 hash160 = BRHTLCScriptPKH(input->script, input->scriptLen, htlcType);
+            hash160 = BRHTLCScriptPKH(input->script, input->scriptLen, htlcType);
             hash = &hash160.u8[0];
         }
 

diff --git a/src/spv/bitcoin/BRWallet.cpp b/src/spv/bitcoin/BRWallet.cpp
@@ -980,7 +980,7 @@ BRTransaction *BRWalletCreateTxForOutputs(BRWallet *wallet, const BRTxOutput out
         tx = (BRTransaction *)BRSetGet(wallet->allTx, o);
 
         // Exclude HTLC outputs
-        uint8_t* pkh = BRScriptPKH(tx->outputs[o->n].script, tx->outputs[o->n].scriptLen);
+        const uint8_t* pkh = BRScriptPKH(tx->outputs[o->n].script, tx->outputs[o->n].scriptLen);
         UInt160 hash160;
         UIntConvert(pkh, hash160);
 

diff --git a/src/spv/spv_wrapper.cpp b/src/spv/spv_wrapper.cpp
@@ -776,6 +776,14 @@ HTLCDetails GetHTLCDetails(CScript& redeemScript)
 {
     HTLCDetails script;
 
+    uint32_t scriptSize{1 /* OP_IF */ + 1 /* OP_SHA256 */ + 1 /* seed size */ + 32 /* seed */ + 1 /* OP_EQUALVERIFY */ + 1 /* seller size */};
+
+    if (redeemScript.size() < scriptSize)
+    {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Incorrect redeemscript length");
+    }
+
+    // Check size field is 32 for seed.
     if (redeemScript[2] != 32)
     {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Incorrect seed hash length");
@@ -788,6 +796,12 @@ HTLCDetails GetHTLCDetails(CScript& redeemScript)
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Seller pubkey incorrect pubkey length");
     }
 
+    scriptSize += sellerLength + 1 /* OP_ELSE */ + 1 /* time size */;
+    if (redeemScript.size() < scriptSize)
+    {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Incorrect redeemscript length");
+    }
+
     script.sellerKey.Set(&redeemScript[37], &redeemScript[37] + sellerLength);
     if (!script.sellerKey.IsValid())
     {
@@ -820,12 +834,25 @@ HTLCDetails GetHTLCDetails(CScript& redeemScript)
         }
     }
 
+    scriptSize += timeoutLength + 1 /* OP_CHECKSEQUENCEVERIFY */ + 1 /* OP_DROP */ + 1 /* buyer size*/;
+    if (redeemScript.size() < scriptSize)
+    {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Incorrect redeemscript length");
+    }
+
     uint8_t buyerLength = redeemScript[41 + timeoutLength + sellerLength];
     if (buyerLength != CPubKey::PUBLIC_KEY_SIZE && buyerLength != CPubKey::COMPRESSED_PUBLIC_KEY_SIZE)
     {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Buyer pubkey incorrect pubkey length");
     }
 
+    // Check redeemscript size is now exact
+    scriptSize += buyerLength + 1 /* OP_ENDIF */ + 1 /* OP_CHECKSIG */;
+    if (redeemScript.size() != scriptSize)
+    {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Incorrect redeemscript length");
+    }
+
     script.buyerKey.Set(&redeemScript[42 + timeoutLength + sellerLength], &redeemScript[42 + timeoutLength + sellerLength] + buyerLength);
     if (!script.buyerKey.IsValid())
     {

diff --git a/test/functional/feature_bitcoin_htlc.py b/test/functional/feature_bitcoin_htlc.py
@@ -206,5 +206,42 @@ def run_test(self):
         assert_equal(len(output[1]['spent']), 2)
         assert_equal(len(output[2]['spent']), 2)
 
+        # Test HTLC script decoding failures
+
+        # Incorrect seed length
+        try:
+            self.nodes[0].spv_decodehtlcscript("63a821df95183883789f237977543885e1f82ddc045a3ba90c8f25b43a5b797a35d20e88210224e7de2f3a9d4cdc4fdc14601c75176287297c212aae9091404956955f1aea86675ab27521035fb3eadde611a39036e61d4c8288d1b896f2c94cee49e60a3d1c02236f4be49068ac")
+        except JSONRPCException as e:
+            errorString = e.error['message']
+            assert("Incorrect seed hash length" in errorString)
+
+        # Incorrect seller pubkey length
+        try:
+            self.nodes[0].spv_decodehtlcscript("63a820df95183883789f237977543885e1f82ddc045a3ba90c8f25b43a5b797a35d20e88200224e7de2f3a9d4cdc4fdc14601c75176287297c212aae9091404956955f1aea86675ab27521035fb3eadde611a39036e61d4c8288d1b896f2c94cee49e60a3d1c02236f4be49068ac")
+        except JSONRPCException as e:
+            errorString = e.error['message']
+            assert("Seller pubkey incorrect pubkey length" in errorString)
+
+        # Incorrect time length
+        try:
+            self.nodes[0].spv_decodehtlcscript("63a820df95183883789f237977543885e1f82ddc045a3ba90c8f25b43a5b797a35d20e88210224e7de2f3a9d4cdc4fdc14601c75176287297c212aae9091404956955f1aea866750b27521035fb3eadde611a39036e61d4c8288d1b896f2c94cee49e60a3d1c02236f4be49068")
+        except JSONRPCException as e:
+            errorString = e.error['message']
+            assert("Incorrect timeout length" in errorString)
+
+        # Incorrect buyer pubkey length
+        try:
+            self.nodes[0].spv_decodehtlcscript("63a820df95183883789f237977543885e1f82ddc045a3ba90c8f25b43a5b797a35d20e88210224e7de2f3a9d4cdc4fdc14601c75176287297c212aae9091404956955f1aea86675ab27521035fb3eadde611a39036e61d4c8288d1b896f2c94cee49e60a3d1c02236f4be49068ac")
+        except JSONRPCException as e:
+            errorString = e.error['message']
+            assert("Buyer pubkey incorrect pubkey length" in errorString)
+
+        # Incorrect redeemscript length
+        try:
+            self.nodes[0].spv_decodehtlcscript("63a820df95183883789f237977543885e1f82ddc045a3ba90c8f25b43a5b797a35d20e88210224e7de2f3a9d4cdc4fdc14601c75176287297c212aae9091404956955f1aea86675ab27521035fb3eadde611a39036e61d4c8288d1b896f2c94cee49e60a3d1c02236f4be49068")
+        except JSONRPCException as e:
+            errorString = e.error['message']
+            assert("Incorrect redeemscript length" in errorString)
+
 if __name__ == '__main__':
     BitcoinHTLCTests().main()