Skip to content

Commit 5ca8ef2

Browse files
committed
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.
1 parent c587577 commit 5ca8ef2

File tree

3 files changed

+21
-5
lines changed

3 files changed

+21
-5
lines changed

src/script/bitcoinconsensus.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,19 @@ struct ECCryptoClosure
6969
ECCryptoClosure instance_of_eccryptoclosure;
7070
}
7171

72+
/** Check that all specified flags are part of the libconsensus interface. */
73+
static bool verify_flags(unsigned int flags)
74+
{
75+
return (flags & ~(bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL)) == 0;
76+
}
77+
7278
static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptPubKeyLen, CAmount amount,
7379
const unsigned char *txTo , unsigned int txToLen,
7480
unsigned int nIn, unsigned int flags, bitcoinconsensus_error* err)
7581
{
82+
if (!verify_flags(flags)) {
83+
return bitcoinconsensus_ERR_INVALID_FLAGS;
84+
}
7685
try {
7786
TxInputStream stream(SER_NETWORK, PROTOCOL_VERSION, txTo, txToLen);
7887
CTransaction tx;

src/script/bitcoinconsensus.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ typedef enum bitcoinconsensus_error_t
4242
bitcoinconsensus_ERR_TX_SIZE_MISMATCH,
4343
bitcoinconsensus_ERR_TX_DESERIALIZE,
4444
bitcoinconsensus_ERR_AMOUNT_REQUIRED,
45+
bitcoinconsensus_ERR_INVALID_FLAGS,
4546
} bitcoinconsensus_error;
4647

4748
/** Script verification flags */
@@ -54,6 +55,9 @@ enum
5455
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKLOCKTIMEVERIFY = (1U << 9), // enable CHECKLOCKTIMEVERIFY (BIP65)
5556
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKSEQUENCEVERIFY = (1U << 10), // enable CHECKSEQUENCEVERIFY (BIP112)
5657
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS = (1U << 11), // enable WITNESS (BIP141)
58+
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL = bitcoinconsensus_SCRIPT_FLAGS_VERIFY_P2SH | bitcoinconsensus_SCRIPT_FLAGS_VERIFY_DERSIG |
59+
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_NULLDUMMY | bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKLOCKTIMEVERIFY |
60+
bitcoinconsensus_SCRIPT_FLAGS_VERIFY_CHECKSEQUENCEVERIFY | bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS
5761
};
5862

5963
/// Returns 1 if the input nIn of the serialized transaction pointed to by

src/test/script_tests.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,14 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript
173173
#if defined(HAVE_CONSENSUS_LIB)
174174
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
175175
stream << tx2;
176-
if (flags & bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS) {
177-
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);
178-
} else {
179-
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);
180-
BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script(begin_ptr(scriptPubKey), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), 0, flags, NULL) == expect,message);
176+
int libconsensus_flags = flags & bitcoinconsensus_SCRIPT_FLAGS_VERIFY_ALL;
177+
if (libconsensus_flags == flags) {
178+
if (flags & bitcoinconsensus_SCRIPT_FLAGS_VERIFY_WITNESS) {
179+
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);
180+
} else {
181+
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);
182+
BOOST_CHECK_MESSAGE(bitcoinconsensus_verify_script(begin_ptr(scriptPubKey), scriptPubKey.size(), (const unsigned char*)&stream[0], stream.size(), 0, libconsensus_flags, NULL) == expect,message);
183+
}
181184
}
182185
#endif
183186
}

0 commit comments

Comments
 (0)