From 5ca8ef299a08aae91d5061750533694b58d810b2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 20 Oct 2016 08:30:03 +0200 Subject: [PATCH] libconsensus: Add input validation of flags Makes it an error to use flags that have not been defined on the libconsensus API. There has been some confusion as to what pass to libconsensus, and (combined with mention in the release notes) this should clear it up. Using undocumented flags is a risk because their meaning, and what combinations are allowed, changes from release to release. E.g. it is no longer possible to pass (CLEANSTACK | P2SH) without running into an assertion after the segwit changes. --- src/script/bitcoinconsensus.cpp | 9 +++++++++ src/script/bitcoinconsensus.h | 4 ++++ src/test/script_tests.cpp | 13 ++++++++----- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index b629f4278b..1d0ca0c5ac 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -69,10 +69,19 @@ struct ECCryptoClosure ECCryptoClosure instance_of_eccryptoclosure; } +/** Check that all specified flags are part of the libconsensus interface. */ +static bool verify_flags(unsigned int flags) +{ + return (flags & ~(bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL)) == 0; +} + static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptPubKeyLen, CAmount amount, const unsigned char *txTo , unsigned int txToLen, unsigned int nIn, unsigned int flags, bitcoinconsensus_error* err) { + if (!verify_flags(flags)) { + return bitcoinconsensus_ERR_INVALID_FLAGS; + } try { TxInputStream stream(SER_NETWORK, PROTOCOL_VERSION, txTo, txToLen); CTransaction tx; diff --git a/src/script/bitcoinconsensus.h b/src/script/bitcoinconsensus.h index 1d2d5c23e4..1bef4fe9e9 100644 --- a/src/script/bitcoinconsensus.h +++ b/src/script/bitcoinconsensus.h @@ -42,6 +42,7 @@ typedef enum bitcoinconsensus_error_t bitcoinconsensus_ERR_TX_SIZE_MISMATCH, bitcoinconsensus_ERR_TX_DESERIALIZE, bitcoinconsensus_ERR_AMOUNT_REQUIRED, + bitcoinconsensus_ERR_INVALID_FLAGS, } bitcoinconsensus_error; /** Script verification flags */ @@ -54,6 +55,9 @@ enum bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKLOCKTIMEVERIFY = (1U << 9), // enable CHECKLOCKTIMEVERIFY (BIP65) bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKSEQUENCEVERIFY = (1U << 10), // enable CHECKSEQUENCEVERIFY (BIP112) bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS = (1U << 11), // enable WITNESS (BIP141) + bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL = bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH | bitcoinconsensus_SCRIPT_FLAGS_VERIFY_DERSIG | + bitcoinconsensus_SCRIPT_FLAGS_VERIFY_NULLDUMMY | bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKLOCKTIMEVERIFY | + bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKSEQUENCEVERIFY | bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS }; /// Returns 1 if the input nIn of the serialized transaction pointed to by diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 561adb8ea2..3169afb13a 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -173,11 +173,14 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript #if defined(HAVE_CONSENSUS_LIB) CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << tx2; - if (flags & bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS) { - BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script_with_amount(begin_ptr(scriptPubKey), scriptPubKey.size(), txCredit.vout[0].nValue, (const unsigned char*)&stream[0], stream.size(), 0, flags, NULL) == expect, message); - } else { - BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script_with_amount(begin_ptr(scriptPubKey), scriptPubKey.size(), 0, (const unsigned char*)&stream[0], stream.size(), 0, flags, NULL) == expect, message); - BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script(begin_ptr(scriptPubKey), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), 0, flags, NULL) == expect,message); + int libconsensus_flags = flags & bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL; + if (libconsensus_flags == flags) { + if (flags & bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS) { + BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script_with_amount(begin_ptr(scriptPubKey), scriptPubKey.size(), txCredit.vout[0].nValue, (const unsigned char*)&stream[0], stream.size(), 0, libconsensus_flags, NULL) == expect, message); + } else { + BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script_with_amount(begin_ptr(scriptPubKey), scriptPubKey.size(), 0, (const unsigned char*)&stream[0], stream.size(), 0, libconsensus_flags, NULL) == expect, message); + BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script(begin_ptr(scriptPubKey), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), 0, libconsensus_flags, NULL) == expect,message); + } } #endif }