diff options
| -rw-r--r-- | src/consensus/tx_check.cpp | 2 | ||||
| -rw-r--r-- | src/net_processing.cpp | 2 | ||||
| -rw-r--r-- | src/script/interpreter.cpp | 4 | ||||
| -rw-r--r-- | src/test/data/script_tests.json | 7 | ||||
| -rw-r--r-- | src/validation.cpp | 3 | ||||
| -rwxr-xr-x | test/functional/feature_block.py | 2 | ||||
| -rwxr-xr-x | test/functional/mempool_accept.py | 1 | ||||
| -rwxr-xr-x | test/functional/p2p_invalid_block.py | 5 |
8 files changed, 16 insertions, 10 deletions
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index 23ed3ecb53..00ebbbd1ab 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -18,7 +18,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR > MAX_BLOCK_WEIGHT) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-txns-oversize"); - // Check for negative or overflow output values + // Check for negative or overflow output values (see CVE-2010-5139) CAmount nValueOut = 0; for (const auto& txout : tx.vout) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a3b865a69e..5c66c4022a 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2513,7 +2513,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } AddOrphanTx(ptx, pfrom->GetId()); - // DoS prevention: do not allow mapOrphanTransactions to grow unbounded + // DoS prevention: do not allow mapOrphanTransactions to grow unbounded (see CVE-2012-3789) unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); unsigned int nEvicted = LimitOrphanTxSize(nMaxOrphanTx); if (nEvicted > 0) { diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 95b25b4911..a819a65d24 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -334,7 +334,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript& opcode == OP_MOD || opcode == OP_LSHIFT || opcode == OP_RSHIFT) - return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes. + return set_error(serror, SCRIPT_ERR_DISABLED_OPCODE); // Disabled opcodes (CVE-2010-5137). // With SCRIPT_VERIFY_CONST_SCRIPTCODE, OP_CODESEPARATOR in non-segwit script is rejected even in an unexecuted branch if (opcode == OP_CODESEPARATOR && sigversion == SigVersion::BASE && (flags & SCRIPT_VERIFY_CONST_SCRIPTCODE)) @@ -1483,6 +1483,8 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, const C return set_error(serror, SCRIPT_ERR_SIG_PUSHONLY); } + // scriptSig and scriptPubKey must be evaluated sequentially on the same stack + // rather than being simply concatenated (see CVE-2010-5141) std::vector<std::vector<unsigned char> > stack, stackCopy; if (!EvalScript(stack, scriptSig, flags, checker, SigVersion::BASE, serror)) // serror is set diff --git a/src/test/data/script_tests.json b/src/test/data/script_tests.json index 9b320b6943..3241f32f56 100644 --- a/src/test/data/script_tests.json +++ b/src/test/data/script_tests.json @@ -829,15 +829,16 @@ ["NOP", "2SWAP 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], ["1", "2 3 2SWAP 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], + +["NOP", "SIZE 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], + +["TEST DISABLED OP CODES (CVE-2010-5137)"], ["'a' 'b'", "CAT", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"], ["'a' 'b' 0", "IF CAT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "CAT disabled"], ["'abc' 1 1", "SUBSTR", "P2SH,STRICTENC", "DISABLED_OPCODE", "SUBSTR disabled"], ["'abc' 1 1 0", "IF SUBSTR ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "SUBSTR disabled"], ["'abc' 2 0", "IF LEFT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "LEFT disabled"], ["'abc' 2 0", "IF RIGHT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "RIGHT disabled"], - -["NOP", "SIZE 1", "P2SH,STRICTENC", "INVALID_STACK_OPERATION"], - ["'abc'", "IF INVERT ELSE 1 ENDIF", "P2SH,STRICTENC", "DISABLED_OPCODE", "INVERT disabled"], ["1 2 0 IF AND ELSE 1 ENDIF", "NOP", "P2SH,STRICTENC", "DISABLED_OPCODE", "AND disabled"], ["1 2 0 IF OR ELSE 1 ENDIF", "NOP", "P2SH,STRICTENC", "DISABLED_OPCODE", "OR disabled"], diff --git a/src/validation.cpp b/src/validation.cpp index 436c62261b..d0ce3f78fc 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1865,7 +1865,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // If such overwrites are allowed, coinbases and transactions depending upon those // can be duplicated to remove the ability to spend the first instance -- even after // being sent to another address. - // See BIP30 and http://r6.ca/blog/20120206T005236Z.html for more information. + // See BIP30, CVE-2012-1909, and http://r6.ca/blog/20120206T005236Z.html for more information. // This logic is not necessary for memory pool transactions, as AcceptToMemoryPool // already refuses previously-known transaction ids entirely. // This rule was originally applied to all blocks with a timestamp after March 15, 2012, 0:00 UTC. @@ -3136,6 +3136,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "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)) return state.Invalid(state.GetReason(), false, state.GetRejectCode(), state.GetRejectReason(), diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 3ad83cd2b3..ce353b2272 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -787,7 +787,7 @@ class FullBlockTest(BitcoinTestFramework): # # Blocks are not allowed to contain a transaction whose id matches that of an earlier, # not-fully-spent transaction in the same chain. To test, make identical coinbases; - # the second one should be rejected. + # the second one should be rejected. See also CVE-2012-1909. # self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)") self.move_tip(60) diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 2bb5d8ab7d..a94187ab90 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -212,6 +212,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): rawtxs=[tx.serialize().hex()], ) + # The following two validations prevent overflow of the output amounts (see CVE-2010-5139). self.log.info('A transaction with too large output value') tx.deserialize(BytesIO(hex_str_to_bytes(raw_tx_reference))) tx.vout[0].nValue = 21000000 * COIN + 1 diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index 1e0b876593..8ba3cc7d72 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -53,10 +53,11 @@ class InvalidBlockRequestTest(BitcoinTestFramework): block_time = best_block["time"] + 1 # Use merkle-root malleability to generate an invalid block with - # same blockheader. + # same blockheader (CVE-2012-2459). # Manufacture a block with 3 transactions (coinbase, spend of prior # coinbase, spend of that spend). Duplicate the 3rd transaction to # leave merkle root and blockheader unchanged but invalidate the block. + # For more information on merkle-root malleability see src/consensus/merkle.cpp. self.log.info("Test merkle root malleability.") block2 = create_block(tip, create_coinbase(height), block_time) @@ -81,7 +82,7 @@ class InvalidBlockRequestTest(BitcoinTestFramework): node.p2p.send_blocks_and_test([block2], node, success=False, reject_reason='bad-txns-duplicate') - # Check transactions for duplicate inputs + # Check transactions for duplicate inputs (CVE-2018-17144) self.log.info("Test duplicate input block.") block2_orig.vtx[2].vin.append(block2_orig.vtx[2].vin[0]) |