Permalink
Browse files

Accurate sigop/sighash accounting and limits

Adds a ValidationCostTracker class that is passed to
CheckInputs() / CScriptCheck() to keep track of the exact number
of signature operations required to validate a block, and the
exact number of bytes hashed to compute signature hashes.

Also extends CHashWriter to keep track of number of bytes hashed.

Signature operations per block are limited to MAX_BLOCK_SIGOPS
(unchanged at 20,000)

Bytes hashed to compute signatures is limited to MAX_BLOCK_SIGHASH
(1.3 GB in this commit).
  • Loading branch information...
1 parent d86ff54 commit 842dc24b23ad9551c67672660c4cba882c4c840a @gavinandresen gavinandresen committed Jan 22, 2016
Showing with 143 additions and 30 deletions.
  1. +3 −1 src/consensus/consensus.h
  2. +6 −1 src/hash.h
  3. +53 −13 src/main.cpp
  4. +56 −4 src/main.h
  5. +5 −3 src/miner.cpp
  6. +9 −3 src/script/interpreter.cpp
  7. +10 −4 src/script/interpreter.h
  8. +1 −1 src/test/script_P2SH_tests.cpp
@@ -10,8 +10,10 @@
static const unsigned int MAX_BLOCK_SIZE = 2000000;
/** The old block size limit */
static const unsigned int OLD_MAX_BLOCK_SIZE = 1000000;
-/** pre-2MB-fork limit on signature operations in a block */
+/** limit on signature operations in a block */
static const unsigned int MAX_BLOCK_SIGOPS = OLD_MAX_BLOCK_SIZE/50;
+/** limit on number of bytes hashed to compute signatures in a block */
+static const unsigned int MAX_BLOCK_SIGHASH = 1300 * 1000 * 1000; // 1.3 gigabytes
/** Coinbase transaction outputs can only be spent after this number of new blocks (network rule) */
static const int COINBASE_MATURITY = 100;
View
@@ -123,15 +123,17 @@ class CHashWriter
{
private:
CHash256 ctx;
+ size_t nBytesHashed;
public:
int nType;
int nVersion;
- CHashWriter(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {}
+ CHashWriter(int nTypeIn, int nVersionIn) : nBytesHashed(0), nType(nTypeIn), nVersion(nVersionIn) {}
CHashWriter& write(const char *pch, size_t size) {
ctx.Write((const unsigned char*)pch, size);
+ nBytesHashed += size;
return (*this);
}
@@ -141,6 +143,9 @@ class CHashWriter
ctx.Finalize((unsigned char*)&result);
return result;
}
+ size_t GetNumBytesHashed() const {
+ return nBytesHashed;
+ }
template<typename T>
CHashWriter& operator<<(const T& obj) {
View
@@ -1092,7 +1092,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
// Check against previous transactions
// This is done last to help prevent CPU exhaustion denial-of-service attacks.
- if (!CheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true))
+ if (!CheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true, NULL))
{
return error("AcceptToMemoryPool: ConnectInputs failed %s", hash.ToString());
}
@@ -1106,7 +1106,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
// There is a similar check in CreateNewBlock() to prevent creating
// invalid blocks, however allowing such transactions into the mempool
// can be exploited as a DoS attack.
- if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true))
+ if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, NULL))
@jtoomim
jtoomim Feb 9, 2016

This line and the one above are tricky to read because they only includes 7 arguments, but CheckInputs is defined with 8 arguments. Thus, to feeble human minds (like mine), it may seem that the NULL here is being used for pvChecks (the last argument) instead of costTracker (the 7th argument).

This is a preexisting issue, and was not introduced in this commit.

(Note: I deleted a comment I made a few minutes ago stating that the arguments were mixed up.)

{
return error("AcceptToMemoryPool: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString());
}
@@ -1445,14 +1445,23 @@ void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCach
}
bool CScriptCheck::operator()() {
+ if (costTracker && !costTracker->IsWithinLimits())
+ return false; // Don't do any more checks if already past limits
+
const CScript &scriptSig = ptxTo->vin[nIn].scriptSig;
- if (!VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, cacheStore), &error)) {
+ CachingTransactionSignatureChecker checker(ptxTo, nIn, cacheStore);
+ if (!VerifyScript(scriptSig, scriptPubKey, nFlags, checker, &error)) {
return ::error("CScriptCheck(): %s:%d VerifySignature failed: %s", ptxTo->GetHash().ToString(), nIn, ScriptErrorString(error));
}
+ if (costTracker) {
+ if (!costTracker->Update(ptxTo->GetHash(), checker.GetNumSigops(), checker.GetBytesHashed()))
+ return ::error("CScriptCheck(): %s:%d sigop and/or sighash byte limit exceeded",
+ ptxTo->GetHash().ToString(), nIn);
+ }
return true;
}
-bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks)
+bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, ValidationCostTracker* costTracker, std::vector<CScriptCheck> *pvChecks)
{
if (!tx.IsCoinBase())
{
@@ -1521,7 +1530,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
assert(coins);
// Verify signature
- CScriptCheck check(*coins, tx, i, flags, cacheStore);
+ CScriptCheck check(costTracker, *coins, tx, i, flags, cacheStore);
if (pvChecks) {
pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back());
@@ -1533,7 +1542,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
// arguments; if so, don't trigger DoS protection to
// avoid splitting the network between upgraded and
// non-upgraded nodes.
- CScriptCheck check(*coins, tx, i,
+ CScriptCheck check(NULL, *coins, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore);
if (check())
return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError())));
@@ -1906,12 +1915,18 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
CBlockUndo blockundo;
+ int64_t nTimeStart = GetTimeMicros();
+
+ // Pre-fork, legacy sigop counting is used, unlimited resource tracker
+ // Post-fork, accurately counted sigop/sighash limits are used
+ ValidationCostTracker costTracker(MaxBlockSigops(block.nTime), MaxBlockSighash(block.nTime));
+
CCheckQueueControl<CScriptCheck> control(fScriptChecks && nScriptCheckThreads ? &scriptcheckqueue : NULL);
- int64_t nTimeStart = GetTimeMicros();
CAmount nFees = 0;
int nInputs = 0;
- unsigned int nSigOps = 0;
+ uint32_t nSigOps = 0;
+ uint32_t nMaxLegacySigops = MaxLegacySigops(block.nTime);
CDiskTxPos pos(pindex->GetBlockPos(), GetSizeOfCompactSize(block.vtx.size()));
std::vector<std::pair<uint256, CDiskTxPos> > vPos;
vPos.reserve(block.vtx.size());
@@ -1922,7 +1937,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
nInputs += tx.vin.size();
nSigOps += GetLegacySigOpCount(tx);
- if (nSigOps > MAX_BLOCK_SIGOPS)
+ if (nSigOps > nMaxLegacySigops)
return state.DoS(100, error("ConnectBlock(): too many sigops"),
REJECT_INVALID, "bad-blk-sigops");
@@ -1938,15 +1953,16 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
// this is to prevent a "rogue miner" from creating
// an incredibly-expensive-to-validate block.
nSigOps += GetP2SHSigOpCount(tx, view);
- if (nSigOps > MAX_BLOCK_SIGOPS)
+ if (nSigOps > nMaxLegacySigops)
return state.DoS(100, error("ConnectBlock(): too many sigops"),
REJECT_INVALID, "bad-blk-sigops");
}
nFees += view.GetValueIn(tx)-tx.GetValueOut();
std::vector<CScriptCheck> vChecks;
- if (!CheckInputs(tx, state, view, fScriptChecks, flags, false, nScriptCheckThreads ? &vChecks : NULL))
+ if (!CheckInputs(tx, state, view, fScriptChecks, flags, false,
+ &costTracker, nScriptCheckThreads ? &vChecks : NULL))
return false;
control.Add(vChecks);
}
@@ -1972,6 +1988,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin
if (!control.Wait())
return state.DoS(100, false);
+
int64_t nTime2 = GetTimeMicros(); nTimeVerify += nTime2 - nTimeStart;
LogPrint("bench", " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs]\n", nInputs - 1, 0.001 * (nTime2 - nTimeStart), nInputs <= 1 ? 0 : 0.001 * (nTime2 - nTimeStart) / (nInputs-1), nTimeVerify * 0.000001);
@@ -2170,7 +2187,7 @@ void static UpdateTip(CBlockIndex *pindexNew) {
const CBlockIndex* pindex = chainActive.Tip();
for (int i = 0; i < 100 && pindex != NULL; i++)
{
- if (!Block::VersionKnown(pindex->nVersion))
+ if (!CBlock::VersionKnown(pindex->nVersion))
++nUpgraded;
pindex = pindex->pprev;
}
@@ -2805,7 +2822,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
{
nSigOps += GetLegacySigOpCount(tx);
}
- if (nSigOps > MAX_BLOCK_SIGOPS)
+ if (nSigOps > MaxLegacySigops(block.nTime))
return state.DoS(100, error("CheckBlock(): out-of-bounds SigOpCount"),
REJECT_INVALID, "bad-blk-sigops", true);
@@ -5270,6 +5287,29 @@ unsigned int MaxBlockSize(uint32_t nBlockTime)
return MAX_BLOCK_SIZE;
}
+/** Maximum size of a block */
+unsigned int MaxBlockSigops(uint32_t nBlockTime)
+{
+ if (nBlockTime < sizeForkTime.load())
+ return std::numeric_limits<uint32_t>::max(); // Use old way of counting
+ return MAX_BLOCK_SIGOPS;
+}
+/** Maximum size of a block */
+unsigned int MaxBlockSighash(uint32_t nBlockTime)
+{
+ if (nBlockTime < sizeForkTime.load())
+ return std::numeric_limits<uint32_t>::max(); // no limit before
+ return MAX_BLOCK_SIGHASH;
+}
+
+/** Maximum legacy (miscounted) sigops in a block */
+uint32_t MaxLegacySigops(uint32_t nBlockTime)
+{
+ if (nBlockTime < sizeForkTime.load())
+ return MAX_BLOCK_SIGOPS;
+ return std::numeric_limits<uint32_t>::max(); // Use accurately-counted limit
+}
+
uint32_t ForkBits(uint32_t nTime) {
uint32_t bits = 0;
AssertLockHeld(cs_main);
View
@@ -35,8 +35,10 @@
#include <utility>
#include <vector>
+#include <boost/atomic.hpp>
#include <boost/unordered_map.hpp>
+class ValidationCostTracker;
class CBlockIndex;
class CBlockTreeDB;
class CBloomFilter;
@@ -315,7 +317,8 @@ unsigned int GetP2SHSigOpCount(const CTransaction& tx, const CCoinsViewCache& ma
* instead of being performed inline.
*/
bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &view, bool fScriptChecks,
- unsigned int flags, bool cacheStore, std::vector<CScriptCheck> *pvChecks = NULL);
+ unsigned int flags, bool cacheStore, ValidationCostTracker* costTracker,
+ std::vector<CScriptCheck> *pvChecks = NULL);
/** Apply the effects of this transaction on the UTXO set represented by view */
void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, int nHeight);
@@ -343,13 +346,52 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime);
*/
bool CheckFinalTx(const CTransaction &tx, int flags = -1);
+/**
+ * Class that keeps track of number of signature operations
+ * and bytes hashed to compute signature hashes.
+ */
+class ValidationCostTracker
+{
+private:
+ mutable CCriticalSection cs;
+ uint32_t nSigops;
+ const uint32_t nMaxSigops;
+ uint32_t nSighashBytes;
+ const uint32_t nMaxSighashBytes;
+
+public:
+ ValidationCostTracker(uint32_t nMaxSigopsIn, uint32_t nMaxSighashBytesIn) :
+ nSigops(0), nMaxSigops(nMaxSigopsIn),
+ nSighashBytes(0), nMaxSighashBytes(nMaxSighashBytesIn) { }
+
+ bool IsWithinLimits() const {
+ LOCK(cs);
+ return (nSigops <= nMaxSigops && nSighashBytes <= nMaxSighashBytes);
+ }
+ bool Update(const uint256& txid, uint32_t nSigopsIn, uint32_t nSighashBytesIn) {
+ LOCK(cs);
+ nSigops += nSigopsIn;
+ nSighashBytes += nSighashBytesIn;
+ return (nSigops <= nMaxSigops && nSighashBytes <= nMaxSighashBytes);
+ }
+ uint32_t GetSigOps() const {
+ LOCK(cs);
+ return nSigops;
+ }
+ uint32_t GetSighashBytes() const {
+ LOCK(cs);
+ return nSighashBytes;
+ }
+};
+
/**
* Closure representing one script verification
* Note that this stores references to the spending transaction
*/
class CScriptCheck
{
private:
+ ValidationCostTracker* costTracker;
CScript scriptPubKey;
const CTransaction *ptxTo;
unsigned int nIn;
@@ -358,14 +400,15 @@ class CScriptCheck
ScriptError error;
public:
- CScriptCheck(): ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
- CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) :
- scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey),
+ CScriptCheck(): costTracker(NULL), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
+ CScriptCheck(ValidationCostTracker* costTrackerIn, const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) :
+ costTracker(costTrackerIn), scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey),
ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR) { }
bool operator()();
void swap(CScriptCheck &check) {
+ std::swap(costTracker, check.costTracker);
scriptPubKey.swap(check.scriptPubKey);
std::swap(ptxTo, check.ptxTo);
std::swap(nIn, check.nIn);
@@ -495,6 +538,15 @@ extern CBlockTreeDB *pblocktree;
/** Maximum size of a block */
unsigned int MaxBlockSize(uint32_t nBlockTime);
+/** Max accurately-counted sigops in a block */
+uint32_t MaxBlockSigops(uint32_t nBlockTime);
+
+/** Max accurately-counted bytes hashed to compute signatures, per block */
+uint32_t MaxBlockSighash(uint32_t nBlockTime);
+
+/** Maximum number of legacy sigops in a block */
+uint32_t MaxLegacySigops(uint32_t nBlockTime);
+
/** What forks we are expressing support for */
uint32_t ForkBits(uint32_t nTime);
View
@@ -99,6 +99,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
if(!pblocktemplate.get())
return NULL;
CBlock *pblock = &pblocktemplate->block; // pointer for convenience
+ ValidationCostTracker resourceTracker(std::numeric_limits<size_t>::max(), std::numeric_limits<size_t>::max());
// -regtest only: allow overriding block.nVersion with
// -blockversion=N to test forking scenarios
@@ -244,6 +245,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
uint64_t nBlockTx = 0;
int nBlockSigOps = 100;
bool fSortedByFee = (nBlockPrioritySize <= 0);
+ uint32_t nMaxLegacySigops = MaxLegacySigops(pblock->nTime);
TxPriorityCompare comparer(fSortedByFee);
std::make_heap(vecPriority.begin(), vecPriority.end(), comparer);
@@ -265,7 +267,7 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
// Legacy limits on sigOps:
unsigned int nTxSigOps = GetLegacySigOpCount(tx);
- if (nBlockSigOps + nTxSigOps >= MAX_BLOCK_SIGOPS)
+ if (nBlockSigOps + nTxSigOps >= nMaxLegacySigops)
continue;
// Skip free transactions if we're past the minimum block size:
@@ -292,14 +294,14 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
CAmount nTxFees = view.GetValueIn(tx)-tx.GetValueOut();
nTxSigOps += GetP2SHSigOpCount(tx, view);
- if (nBlockSigOps + nTxSigOps >= MAX_BLOCK_SIGOPS)
+ if (nBlockSigOps + nTxSigOps >= nMaxLegacySigops)
continue;
// Note that flags: we don't want to set mempool/IsStandard()
// policy here, but we still have to ensure that the block we
// create only contains transactions that are valid in new blocks.
CValidationState state;
- if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true))
+ if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, &resourceTracker))
continue;
UpdateCoins(tx, state, view, nHeight);
@@ -1075,7 +1075,7 @@ class CTransactionSignatureSerializer {
} // anon namespace
-uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType)
+uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, size_t* nHashedOut)
{
static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
if (nIn >= txTo.vin.size()) {
@@ -1097,6 +1097,8 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig
// Serialize and hash
CHashWriter ss(SER_GETHASH, 0);
ss << txTmp << nHashType;
+ if (nHashedOut != NULL)
+ *nHashedOut = ss.GetNumBytesHashed();
return ss.GetHash();
}
@@ -1105,7 +1107,8 @@ bool TransactionSignatureChecker::VerifySignature(const std::vector<unsigned cha
return pubkey.Verify(sighash, vchSig);
}
-bool TransactionSignatureChecker::CheckSig(const vector<unsigned char>& vchSigIn, const vector<unsigned char>& vchPubKey, const CScript& scriptCode) const
+bool TransactionSignatureChecker::CheckSig(const vector<unsigned char>& vchSigIn, const vector<unsigned char>& vchPubKey,
+ const CScript& scriptCode) const
{
CPubKey pubkey(vchPubKey);
if (!pubkey.IsValid())
@@ -1118,7 +1121,10 @@ bool TransactionSignatureChecker::CheckSig(const vector<unsigned char>& vchSigIn
int nHashType = vchSig.back();
vchSig.pop_back();
- uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType);
+ size_t nHashed = 0;
+ uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, &nHashed);
+ nBytesHashed += nHashed;
+ ++nSigops;
if (!VerifySignature(vchSig, pubkey, sighash))
return false;
Oops, something went wrong.

0 comments on commit 842dc24

Please sign in to comment.