aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--depends/packages/boost.mk5
-rw-r--r--src/consensus/tx_check.cpp19
-rw-r--r--src/consensus/tx_check.h2
-rw-r--r--src/test/compress_tests.cpp73
-rw-r--r--src/test/fuzz/transaction.cpp7
-rw-r--r--src/validation.cpp10
6 files changed, 92 insertions, 24 deletions
diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk
index 5df49b2af8..766b8d224e 100644
--- a/depends/packages/boost.mk
+++ b/depends/packages/boost.mk
@@ -10,15 +10,14 @@ $(package)_config_opts_debug=variant=debug
$(package)_config_opts=--layout=tagged --build-type=complete --user-config=user-config.jam
$(package)_config_opts+=threading=multi link=static -sNO_BZIP2=1 -sNO_ZLIB=1
$(package)_config_opts_linux=threadapi=pthread runtime-link=shared
-$(package)_config_opts_darwin=--toolset=darwin-4.2.1 runtime-link=shared
+$(package)_config_opts_darwin=--toolset=clang-darwin runtime-link=shared
$(package)_config_opts_mingw32=binary-format=pe target-os=windows threadapi=win32 runtime-link=static
$(package)_config_opts_x86_64_mingw32=address-model=64
$(package)_config_opts_i686_mingw32=address-model=32
$(package)_config_opts_i686_linux=address-model=32 architecture=x86
$(package)_toolset_$(host_os)=gcc
$(package)_archiver_$(host_os)=$($(package)_ar)
-$(package)_toolset_darwin=darwin
-$(package)_archiver_darwin=$($(package)_libtool)
+$(package)_toolset_darwin=clang-darwin
$(package)_config_libraries=chrono,filesystem,system,thread,test
$(package)_cxxflags=-std=c++11 -fvisibility=hidden
$(package)_cxxflags_linux=-fPIC
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index 1206035839..6793f871cf 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -7,7 +7,7 @@
#include <primitives/transaction.h>
#include <consensus/validation.h>
-bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
+bool CheckTransaction(const CTransaction& tx, CValidationState& state)
{
// Basic checks that don't depend on any context
if (tx.vin.empty())
@@ -31,14 +31,15 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe
return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge");
}
- // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
- if (fCheckDuplicateInputs) {
- std::set<COutPoint> vInOutPoints;
- for (const auto& txin : tx.vin)
- {
- if (!vInOutPoints.insert(txin.prevout).second)
- return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate");
- }
+ // Check for duplicate inputs (see CVE-2018-17144)
+ // While Consensus::CheckTxInputs does check if all inputs of a tx are available, and UpdateCoins marks all inputs
+ // of a tx as spent, it does not check if the tx has duplicate inputs.
+ // Failure to run this check will result in either a crash or an inflation bug, depending on the implementation of
+ // the underlying coins database.
+ std::set<COutPoint> vInOutPoints;
+ for (const auto& txin : tx.vin) {
+ if (!vInOutPoints.insert(txin.prevout).second)
+ return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate");
}
if (tx.IsCoinBase())
diff --git a/src/consensus/tx_check.h b/src/consensus/tx_check.h
index bcfdf36bf9..6f3f8fe969 100644
--- a/src/consensus/tx_check.h
+++ b/src/consensus/tx_check.h
@@ -15,6 +15,6 @@
class CTransaction;
class CValidationState;
-bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
+bool CheckTransaction(const CTransaction& tx, CValidationState& state);
#endif // BITCOIN_CONSENSUS_TX_CHECK_H
diff --git a/src/test/compress_tests.cpp b/src/test/compress_tests.cpp
index e8f149470e..c6a08b293f 100644
--- a/src/test/compress_tests.cpp
+++ b/src/test/compress_tests.cpp
@@ -4,6 +4,7 @@
#include <compressor.h>
#include <test/setup_common.h>
+#include <script/standard.h>
#include <stdint.h>
@@ -61,4 +62,76 @@ BOOST_AUTO_TEST_CASE(compress_amounts)
BOOST_CHECK(TestDecode(i));
}
+BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id)
+{
+ // case CKeyID
+ CKey key;
+ key.MakeNewKey(true);
+ CPubKey pubkey = key.GetPubKey();
+
+ CScript script = CScript() << OP_DUP << OP_HASH160 << ToByteVector(pubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+ BOOST_CHECK_EQUAL(script.size(), 25);
+
+ std::vector<unsigned char> out;
+ bool done = CompressScript(script, out);
+ BOOST_CHECK_EQUAL(done, true);
+
+ // Check compressed script
+ BOOST_CHECK_EQUAL(out.size(), 21);
+ BOOST_CHECK_EQUAL(out[0], 0x00);
+ BOOST_CHECK_EQUAL(memcmp(&out[1], &script[3], 20), 0); // compare the 20 relevant chars of the CKeyId in the script
+}
+
+BOOST_AUTO_TEST_CASE(compress_script_to_cscript_id)
+{
+ // case CScriptID
+ CScript script, redeemScript;
+ script << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+ BOOST_CHECK_EQUAL(script.size(), 23);
+
+ std::vector<unsigned char> out;
+ bool done = CompressScript(script, out);
+ BOOST_CHECK_EQUAL(done, true);
+
+ // Check compressed script
+ BOOST_CHECK_EQUAL(out.size(), 21);
+ BOOST_CHECK_EQUAL(out[0], 0x01);
+ BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 20), 0); // compare the 20 relevant chars of the CScriptId in the script
+}
+
+BOOST_AUTO_TEST_CASE(compress_script_to_compressed_pubkey_id)
+{
+ CKey key;
+ key.MakeNewKey(true); // case compressed PubKeyID
+
+ CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // COMPRESSED_PUBLIC_KEY_SIZE (33)
+ BOOST_CHECK_EQUAL(script.size(), 35);
+
+ std::vector<unsigned char> out;
+ bool done = CompressScript(script, out);
+ BOOST_CHECK_EQUAL(done, true);
+
+ // Check compressed script
+ BOOST_CHECK_EQUAL(out.size(), 33);
+ BOOST_CHECK_EQUAL(memcmp(&out[0], &script[1], 1), 0);
+ BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0); // compare the 32 chars of the compressed CPubKey
+}
+
+BOOST_AUTO_TEST_CASE(compress_script_to_uncompressed_pubkey_id)
+{
+ CKey key;
+ key.MakeNewKey(false); // case uncompressed PubKeyID
+ CScript script = CScript() << ToByteVector(key.GetPubKey()) << OP_CHECKSIG; // PUBLIC_KEY_SIZE (65)
+ BOOST_CHECK_EQUAL(script.size(), 67); // 1 char code + 65 char pubkey + OP_CHECKSIG
+
+ std::vector<unsigned char> out;
+ bool done = CompressScript(script, out);
+ BOOST_CHECK_EQUAL(done, true);
+
+ // Check compressed script
+ BOOST_CHECK_EQUAL(out.size(), 33);
+ BOOST_CHECK_EQUAL(memcmp(&out[1], &script[2], 32), 0); // first 32 chars of CPubKey are copied into out[1:]
+ BOOST_CHECK_EQUAL(out[0], 0x04 | (script[65] & 0x01)); // least significant bit (lsb) of last char of pubkey is mapped into out[0]
+}
+
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp
index 96d7947b07..383d879040 100644
--- a/src/test/fuzz/transaction.cpp
+++ b/src/test/fuzz/transaction.cpp
@@ -43,12 +43,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
}
CValidationState state_with_dupe_check;
- const bool valid_with_dupe_check = CheckTransaction(tx, state_with_dupe_check, /* fCheckDuplicateInputs= */ true);
- CValidationState state_without_dupe_check;
- const bool valid_without_dupe_check = CheckTransaction(tx, state_without_dupe_check, /* fCheckDuplicateInputs= */ false);
- if (valid_with_dupe_check) {
- assert(valid_without_dupe_check);
- }
+ (void)CheckTransaction(tx, state_with_dupe_check);
const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE};
std::string reason;
diff --git a/src/validation.cpp b/src/validation.cpp
index 70b847d3b0..9301066c6a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -392,7 +392,7 @@ static void UpdateMempoolForReorg(DisconnectedBlockTransactions& disconnectpool,
// Used to avoid mempool polluting consensus critical paths if CCoinsViewMempool
// were somehow broken and returning the wrong scriptPubKeys
static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, const CTxMemPool& pool,
- unsigned int flags, bool cacheSigStore, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
+ unsigned int flags, PrecomputedTransactionData& txdata) EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
AssertLockHeld(cs_main);
// pool.cs should be locked already, but go ahead and re-take the lock here
@@ -422,7 +422,8 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt
}
}
- return CheckInputs(tx, state, view, flags, cacheSigStore, true, txdata);
+ // Call CheckInputs() to cache signature and script validity against current tip consensus rules.
+ return CheckInputs(tx, state, view, flags, /* cacheSigStore = */ true, /* cacheFullSciptStore = */ true, txdata);
}
namespace {
@@ -959,7 +960,7 @@ bool MemPoolAccept::ConsensusScriptChecks(ATMPArgs& args, Workspace& ws, Precomp
// invalid blocks (using TestBlockValidity), however allowing such
// transactions into the mempool can be exploited as a DoS attack.
unsigned int currentBlockScriptVerifyFlags = GetBlockScriptFlags(::ChainActive().Tip(), chainparams.GetConsensus());
- if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, true, txdata)) {
+ if (!CheckInputsFromMempoolAndCache(tx, state, m_view, m_pool, currentBlockScriptVerifyFlags, txdata)) {
return error("%s: BUG! PLEASE REPORT THIS! CheckInputs failed against latest-block but not STANDARD flags %s, %s",
__func__, hash.ToString(), FormatStateMessage(state));
}
@@ -3301,9 +3302,8 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase");
// Check transactions
- // Must check for duplicate inputs (see CVE-2018-17144)
for (const auto& tx : block.vtx)
- if (!CheckTransaction(*tx, state, true))
+ if (!CheckTransaction(*tx, state))
return state.Invalid(state.GetReason(), false, state.GetRejectReason(),
strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage()));