From 7ea0ad539ff83f09cf617a9fb7c4d774e45efdc0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 6 Jun 2015 11:45:35 -0700 Subject: [PATCH 1/5] Fail in DecodeHexTx if there is extra data at the end --- src/core_read.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core_read.cpp b/src/core_read.cpp index a5f232c22..a8d667e3b 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -111,6 +111,8 @@ bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx, bool fTry CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION); try { ssData >> tx; + if (!ssData.empty()) + return false; } catch (const std::exception&) { return false; From 922bea90c274d45b13812a031242964aa15b6c1d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 28 Jan 2017 11:32:34 -0500 Subject: [PATCH 2/5] Better handle invalid parameters to signrawtransaction This silently skips trying to merge signatures from inputs which do not exist from transactions provided to signrawtransaction, instead of hitting an assert. --- src/rpc/rawtransaction.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index c10de45f8..21396ebb0 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -835,7 +835,9 @@ UniValue signrawtransaction(const JSONRPCRequest& request) // ... and merge in other signatures: BOOST_FOREACH(const CMutableTransaction& txv, txVariants) { - sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(txv, i)); + if (txv.vin.size() > i) { + sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(txv, i)); + } } UpdateTransaction(mergedTx, i, sigdata); From 691710a648c54c96c302ebbe666da85cfaada5f9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 2 Feb 2017 15:11:28 -0500 Subject: [PATCH 3/5] [qa] Test that decoderawtransaction throws with extra data appended --- qa/rpc-tests/signrawtransactions.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/qa/rpc-tests/signrawtransactions.py b/qa/rpc-tests/signrawtransactions.py index c61a28061..009f01f79 100755 --- a/qa/rpc-tests/signrawtransactions.py +++ b/qa/rpc-tests/signrawtransactions.py @@ -78,6 +78,16 @@ class SignRawTransactionsTest(BitcoinTestFramework): outputs = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1} rawTx = self.nodes[0].createrawtransaction(inputs, outputs) + + # Make sure decoderawtransaction is at least marginally sane + decodedRawTx = self.nodes[0].decoderawtransaction(rawTx) + for i, inp in enumerate(inputs): + assert_equal(decodedRawTx["vin"][i]["txid"], inp["txid"]) + assert_equal(decodedRawTx["vin"][i]["vout"], inp["vout"]) + + # Make sure decoderawtransaction throws if there is extra data + assert_raises(JSONRPCException, self.nodes[0].decoderawtransaction, rawTx + "00") + rawTxSigned = self.nodes[0].signrawtransaction(rawTx, scripts, privKeys) # 3) The transaction has no complete set of signatures From ec4f7e433e5b1724ca427e73fb264de693e82235 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 2 Feb 2017 16:24:31 -0500 Subject: [PATCH 4/5] [qa] Add second input to signrawtransaction test case --- qa/rpc-tests/signrawtransactions.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/signrawtransactions.py b/qa/rpc-tests/signrawtransactions.py index 009f01f79..1705ab96c 100755 --- a/qa/rpc-tests/signrawtransactions.py +++ b/qa/rpc-tests/signrawtransactions.py @@ -26,12 +26,14 @@ class SignRawTransactionsTest(BitcoinTestFramework): 1) The transaction has a complete set of signatures 2) No script verification error occurred""" - privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N'] + privKeys = ['cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N', 'cVKpPfVKSJxKqVpE9awvXNWuLHCa5j5tiE7K6zbUSptFpTEtiFrA'] inputs = [ - # Valid pay-to-pubkey script + # Valid pay-to-pubkey scripts {'txid': '9b907ef1e3c26fc71fe4a4b3580bc75264112f95050014157059c736f0202e71', 'vout': 0, - 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'} + 'scriptPubKey': '76a91460baa0f494b38ce3c940dea67f3804dc52d1fb9488ac'}, + {'txid': '83a4f6a6b73660e13ee6cb3c6063fa3759c50c9b7521d0536022961898f4fb02', 'vout': 0, + 'scriptPubKey': '76a914669b857c03a5ed269d5d85a1ffac9ed5d663072788ac'}, ] outputs = {'mpLQjfK79b7CCV4VMJWEWAj5Mpx8Up5zxB': 0.1} From 6dbfe08c29535bb7b0359de8fe22acedcbf3532a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 2 Feb 2017 16:24:52 -0500 Subject: [PATCH 5/5] [qa] test signrawtransaction merge with missing inputs --- qa/rpc-tests/signrawtransactions.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/qa/rpc-tests/signrawtransactions.py b/qa/rpc-tests/signrawtransactions.py index 1705ab96c..109312bd5 100755 --- a/qa/rpc-tests/signrawtransactions.py +++ b/qa/rpc-tests/signrawtransactions.py @@ -48,6 +48,22 @@ class SignRawTransactionsTest(BitcoinTestFramework): # 2) No script verification error occurred assert 'errors' not in rawTxSigned + # Check that signrawtransaction doesn't blow up on garbage merge attempts + dummyTxInconsistent = self.nodes[0].createrawtransaction([inputs[0]], outputs) + rawTxUnsigned = self.nodes[0].signrawtransaction(rawTx + dummyTxInconsistent, inputs) + + assert 'complete' in rawTxUnsigned + assert_equal(rawTxUnsigned['complete'], False) + + # Check that signrawtransaction properly merges unsigned and signed txn, even with garbage in the middle + rawTxSigned2 = self.nodes[0].signrawtransaction(rawTxUnsigned["hex"] + dummyTxInconsistent + rawTxSigned["hex"], inputs) + + assert 'complete' in rawTxSigned2 + assert_equal(rawTxSigned2['complete'], True) + + assert 'errors' not in rawTxSigned2 + + def script_verification_error_test(self): """Creates and signs a raw transaction with valid (vin 0), invalid (vin 1) and one missing (vin 2) input script.