Verify all incoming txs unless too big or too much hashing #8593

Open
wants to merge 1 commit into
from
Jump to file or symbol
Failed to load files and symbols.
+55 −21
Split
View
@@ -1026,6 +1026,22 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& in
return nSigOps;
}
+unsigned int GetAccurateBaseSigOpCount(const CTransaction& tx, const CCoinsViewCache& inputs, int flags)
+{
+ if (tx.IsCoinBase())
+ return 0;
+ unsigned int nSigOps = 0;
+ for (unsigned int i = 0; i < tx.vin.size(); i++)
+ {
+ nSigOps += tx.vin[i].scriptSig.GetSigOpCount(true);
+ const CTxOut &prevout = inputs.GetOutputFor(tx.vin[i]);
+ nSigOps += prevout.scriptPubKey.GetSigOpCount(true);
+ if (flags & SCRIPT_VERIFY_P2SH && prevout.scriptPubKey.IsPayToScriptHash())
+ nSigOps += prevout.scriptPubKey.GetSigOpCount(tx.vin[i].scriptSig);
+ }
+ return nSigOps;
+}
+
int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& inputs, int flags)
{
int64_t nSigOps = GetLegacySigOpCount(tx) * WITNESS_SCALE_FACTOR;
@@ -1260,6 +1276,34 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final");
}
+ int64_t nSigOpsHashing = GetWitnessStrippedTransactionWeight(tx) * GetAccurateBaseSigOpCount(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS) / WITNESS_SCALE_FACTOR;
+ if (fRequireStandard && nSigOpsHashing > MAX_STANDARD_TX_SIGOPS_HASHING)
+ return state.DoS(0, false, REJECT_NONSTANDARD, "bad-txns-too-many-sigops-hashing", false,
+ strprintf("%d", nSigOpsHashing));
+
+ unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS;
+ if (!Params().RequireStandard()) {
+ scriptVerifyFlags = GetArg("-promiscuousmempoolflags", scriptVerifyFlags);
+ }
+
+ // Check against previous transactions
+ PrecomputedTransactionData txdata(tx);
+ if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, txdata)) {
+ // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we
+ // need to turn both off, and compare against just turning off CLEANSTACK
+ // to see if the failure is specifically due to witness validation.
+ if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) &&
+ !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, txdata)) {
+ // Only the witness is wrong, so the transaction itself may be fine.
+ state.SetCorruptionPossible();
+ }
+ return false;
+ }
+
+ // Now we know the witness is valid. We could check the size with witness
+ if (fRequireStandard && GetTransactionWeight(tx) >= MAX_STANDARD_TX_WEIGHT)
@instagibbs

instagibbs Aug 29, 2016

Member

can't a relay node stuff in a too-large valid witness causing this to enter rejection cache?

@sipa

sipa Aug 29, 2016

Owner

No, because the CheckInputs above will catch that as mauling.

@jl2012

jl2012 Aug 29, 2016

Member

@sipa: it is possible in some cases. That's why we need NULLDUMMY (#8533) and MINIMALIF (#8528)

@dcousens

dcousens Aug 30, 2016

Contributor

@sipa in what context do you refer to mauling? Mutation/malleability?

@sipa

sipa Aug 30, 2016

Owner

@dcousens I've found some literature that uses 'to maul' as the verb related to malleability, and not 'to malleate', so I guess we should use that :)

+ return state.DoS(0, false, REJECT_NONSTANDARD, "tx-size");
+
// Check for non-standard pay-to-script-hash in inputs
if (fRequireStandard && !AreInputsStandard(tx, view))
return state.Invalid(false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs");
@@ -1489,26 +1533,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C
}
}
- unsigned int scriptVerifyFlags = STANDARD_SCRIPT_VERIFY_FLAGS;
- if (!Params().RequireStandard()) {
- scriptVerifyFlags = GetArg("-promiscuousmempoolflags", scriptVerifyFlags);
- }
-
- // Check against previous transactions
- // This is done last to help prevent CPU exhaustion denial-of-service attacks.
- PrecomputedTransactionData txdata(tx);
- if (!CheckInputs(tx, state, view, true, scriptVerifyFlags, true, txdata)) {
- // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we
- // need to turn both off, and compare against just turning off CLEANSTACK
- // to see if the failure is specifically due to witness validation.
- if (CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) &&
- !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, txdata)) {
- // Only the witness is wrong, so the transaction itself may be fine.
- state.SetCorruptionPossible();
- }
- return false;
- }
-
// Check again against just the consensus-critical mandatory script
// verification flags, in case of bugs in the standard flags that cause
// transactions to pass as valid when they're actually invalid. For
View
@@ -67,7 +67,7 @@ bool IsStandardTx(const CTransaction& tx, std::string& reason, const bool witnes
// almost as much to process as they cost the sender in fees, because
// computing signature hashes is O(ninputs*txsize). Limiting transactions
// to MAX_STANDARD_TX_SIZE mitigates CPU exhaustion attacks.
- unsigned int sz = GetTransactionWeight(tx);
+ unsigned int sz = GetWitnessStrippedTransactionWeight(tx);
if (sz >= MAX_STANDARD_TX_WEIGHT) {
reason = "tx-size";
return false;
View
@@ -30,6 +30,8 @@ static const unsigned int MAX_STANDARD_TX_SIGOPS_COST = MAX_BLOCK_SIGOPS_COST/5;
static const unsigned int DEFAULT_MAX_MEMPOOL_SIZE = 300;
/** Default for -bytespersigop */
static const unsigned int DEFAULT_BYTES_PER_SIGOP = 20;
+/** Maximum amount of estimated hashing in base CHECKSIG operations */
@instagibbs

instagibbs Aug 29, 2016

Member

in bytes?

@jl2012

jl2012 Aug 29, 2016

Member

yes, 10MB

@instagibbs

instagibbs Aug 29, 2016

Member

please add units in comment thanks

+static const unsigned int MAX_STANDARD_TX_SIGOPS_HASHING = 10000000;
/**
* Standard script verification flags that standard transactions will comply
* with. However scripts violating these flags may still be present in valid
@@ -153,3 +153,8 @@ int64_t GetTransactionWeight(const CTransaction& tx)
{
return ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR -1) + ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION);
}
+
+int64_t GetWitnessStrippedTransactionWeight(const CTransaction& tx)
+{
+ return ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * WITNESS_SCALE_FACTOR;
+}
@instagibbs

instagibbs Aug 29, 2016

Member

nit: add newline here

@@ -464,4 +464,7 @@ struct CMutableTransaction
/** Compute the weight of a transaction, as defined by BIP 141 */
int64_t GetTransactionWeight(const CTransaction &tx);
+// Compute the weight of a transaction without witness
+int64_t GetWitnessStrippedTransactionWeight(const CTransaction &tx);
+
#endif // BITCOIN_PRIMITIVES_TRANSACTION_H