From 9b59e3bda8c137bff885db5b1f9150346e36e076 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Mon, 2 Dec 2013 11:33:44 -0800 Subject: [PATCH] Sanitize assert usage and refuse to compile with NDEBUG. There were quite a few places where assert() was used with side effects, making operation with NDEBUG non-functional. This commit fixes all the cases I know about, but also adds an #error on NDEBUG because the code is untested without assertions and may still have vulnerabilities if used without assert. --- src/key.cpp | 7 +++++-- src/main.cpp | 19 +++++++++++++++---- src/wallet.cpp | 4 +++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/key.cpp b/src/key.cpp index 2fd68fa56..b57b7c506 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -148,10 +148,13 @@ public: } void SetSecretBytes(const unsigned char vch[32]) { + bool ret; BIGNUM bn; BN_init(&bn); - assert(BN_bin2bn(vch, 32, &bn)); - assert(EC_KEY_regenerate_key(pkey, &bn)); + ret = BN_bin2bn(vch, 32, &bn); + assert(ret); + ret = EC_KEY_regenerate_key(pkey, &bn); + assert(ret); BN_clear_free(&bn); } diff --git a/src/main.cpp b/src/main.cpp index eb3af5bea..25201c736 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -27,6 +27,10 @@ using namespace std; using namespace boost; +#if defined(NDEBUG) +# error "Bitcoin cannot be compiled without assertions." +#endif + // // Global state // @@ -1266,18 +1270,21 @@ void UpdateTime(CBlockHeader& block, const CBlockIndex* pindexPrev) void UpdateCoins(const CTransaction& tx, CValidationState &state, CCoinsViewCache &inputs, CTxUndo &txundo, int nHeight, const uint256 &txhash) { + bool ret; // mark inputs spent if (!tx.IsCoinBase()) { BOOST_FOREACH(const CTxIn &txin, tx.vin) { CCoins &coins = inputs.GetCoins(txin.prevout.hash); CTxInUndo undo; - assert(coins.Spend(txin.prevout, undo)); + ret = coins.Spend(txin.prevout, undo); + assert(ret); txundo.vprevout.push_back(undo); } } // add outputs - assert(inputs.SetCoins(txhash, CCoins(tx, nHeight))); + ret = inputs.SetCoins(txhash, CCoins(tx, nHeight)); + assert(ret); } bool CScriptCheck::operator()() const { @@ -1651,7 +1658,9 @@ bool ConnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex, C return state.Abort(_("Failed to write transaction index")); // add this block to the view's block chain - assert(view.SetBestBlock(pindex->GetBlockHash())); + bool ret; + ret = view.SetBestBlock(pindex->GetBlockHash()); + assert(ret); // Watch for transactions paying to me for (unsigned int i = 0; i < block.vtx.size(); i++) @@ -1746,7 +1755,9 @@ bool SetBestChain(CValidationState &state, CBlockIndex* pindexNew) // Flush changes to global coin state int64_t nStart = GetTimeMicros(); int nModified = view.GetCacheSize(); - assert(view.Flush()); + bool ret; + ret = view.Flush(); + assert(ret); int64_t nTime = GetTimeMicros() - nStart; if (fBenchmark) LogPrintf("- Flush %i transactions: %.2fms (%.4fms/tx)\n", nModified, 0.001 * nTime, 0.001 * nTime / nModified); diff --git a/src/wallet.cpp b/src/wallet.cpp index b9110d127..14d685d6e 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -1298,7 +1298,9 @@ bool CWallet::CreateTransaction(const vector >& vecSend, // Reserve a new key pair from key pool CPubKey vchPubKey; - assert(reservekey.GetReservedKey(vchPubKey)); // should never fail, as we just unlocked + bool ret; + ret = reservekey.GetReservedKey(vchPubKey); + assert(ret); // should never fail, as we just unlocked scriptChange.SetDestination(vchPubKey.GetID()); }