From 5b75c477841fa463aad9c6e6d95a98b50ce14dd3 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 30 May 2017 15:42:10 -0700 Subject: [PATCH 1/2] Add a valid opcode sanity check to CScript Added a function in CScript that checks if the script contains valid opcodes. Add a test for that function --- src/script/script.cpp | 12 ++++++++++++ src/script/script.h | 3 +++ src/test/script_tests.cpp | 14 ++++++++++++++ 3 files changed, 29 insertions(+) diff --git a/src/script/script.cpp b/src/script/script.cpp index 70eb8a139..a71fee19c 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -267,3 +267,15 @@ std::string CScriptWitness::ToString() const } return ret + ")"; } + +bool CScript::HasValidOps() const +{ + CScript::const_iterator it = begin(); + while (it < end()) { + opcodetype opcode; + if (!GetOp(it, opcode) || opcode > 0xb9) { + return false; + } + } + return true; +} diff --git a/src/script/script.h b/src/script/script.h index 95a5999a1..25b80ef62 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -630,6 +630,9 @@ public: bool IsPushOnly(const_iterator pc) const; bool IsPushOnly() const; + /** Check if the script contains valid OP_CODES */ + bool HasValidOps() const; + /** * Returns whether the script is guaranteed to fail at execution, * regardless of the initial stack. This allows outputs to be pruned diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 343c645cb..70544cacd 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1438,4 +1438,18 @@ BOOST_AUTO_TEST_CASE(script_FindAndDelete) BOOST_CHECK(s == expect); } +BOOST_AUTO_TEST_CASE(script_HasValidOps) +{ + // Exercise the HasValidOps functionality + CScript script; + script = ScriptFromHex("76a9141234567890abcdefa1a2a3a4a5a6a7a8a9a0aaab88ac"); // Normal script + BOOST_CHECK(script.HasValidOps()); + script = ScriptFromHex("76a914ff34567890abcdefa1a2a3a4a5a6a7a8a9a0aaab88ac"); + BOOST_CHECK(script.HasValidOps()); + script = ScriptFromHex("ff88ac"); // Script with OP_INVALIDOPCODE explicit + BOOST_CHECK(!script.HasValidOps()); + script = ScriptFromHex("88acc0"); // Script with undefined opcode + BOOST_CHECK(!script.HasValidOps()); +} + BOOST_AUTO_TEST_SUITE_END() From ac4e438229134595e949bfedb1f487c71fd45d24 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Tue, 30 May 2017 15:43:07 -0700 Subject: [PATCH 2/2] Sanity check transaction scripts in DecodeHexTx Make sure that the scripts of decoded transactions are valid scripts. --- src/core_read.cpp | 29 ++++++++++++++++++++++++++--- src/script/script.cpp | 3 ++- src/script/script.h | 3 +++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/core_read.cpp b/src/core_read.cpp index a8d667e3b..463871d17 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -88,10 +88,32 @@ CScript ParseScript(const std::string& s) return result; } +// Check that all of the input and output scripts of a transaction contains valid opcodes +bool CheckTxScriptsSanity(const CMutableTransaction& tx) +{ + // Check input scripts for non-coinbase txs + if (!CTransaction(tx).IsCoinBase()) { + for (unsigned int i = 0; i < tx.vin.size(); i++) { + if (!tx.vin[i].scriptSig.HasValidOps() || tx.vin[i].scriptSig.size() > MAX_SCRIPT_SIZE) { + return false; + } + } + } + // Check output scripts + for (unsigned int i = 0; i < tx.vout.size(); i++) { + if (!tx.vout[i].scriptPubKey.HasValidOps() || tx.vout[i].scriptPubKey.size() > MAX_SCRIPT_SIZE) { + return false; + } + } + + return true; +} + bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx, bool fTryNoWitness) { - if (!IsHex(strHexTx)) + if (!IsHex(strHexTx)) { return false; + } std::vector txData(ParseHex(strHexTx)); @@ -99,7 +121,7 @@ bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx, bool fTry CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS); try { ssData >> tx; - if (ssData.eof()) { + if (ssData.eof() && CheckTxScriptsSanity(tx)) { return true; } } @@ -111,8 +133,9 @@ bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx, bool fTry CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION); try { ssData >> tx; - if (!ssData.empty()) + if (!ssData.empty()) { return false; + } } catch (const std::exception&) { return false; diff --git a/src/script/script.cpp b/src/script/script.cpp index a71fee19c..a10b619f7 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -273,7 +273,8 @@ bool CScript::HasValidOps() const CScript::const_iterator it = begin(); while (it < end()) { opcodetype opcode; - if (!GetOp(it, opcode) || opcode > 0xb9) { + std::vector item; + if (!GetOp(it, opcode, item) || opcode > MAX_OPCODE || item.size() > MAX_SCRIPT_ELEMENT_SIZE) { return false; } } diff --git a/src/script/script.h b/src/script/script.h index 25b80ef62..23706b982 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -190,6 +190,9 @@ enum opcodetype OP_INVALIDOPCODE = 0xff, }; +// Maximum value that an opcode can be +static const unsigned int MAX_OPCODE = OP_NOP10; + const char* GetOpName(opcodetype opcode); class scriptnum_error : public std::runtime_error