From b5173fd6e5d0a2a2ea1f52540b570db035d76f3b Mon Sep 17 00:00:00 2001 From: CryptoManiac Date: Sun, 8 May 2016 00:26:54 +0300 Subject: [PATCH] Use CBigNum for temporary variables to prevent overflow. Which is unlikely to happen, though. --- src/bignum.h | 5 +++ src/main.cpp | 85 ++++++++++++++++++++++++++++--------------------------- src/main.h | 7 ++-- src/wallet.cpp | 51 ++++++++++++++++++--------------- 4 files changed, 80 insertions(+), 68 deletions(-) diff --git a/src/bignum.h b/src/bignum.h index 4bda99d..441c960 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -191,6 +191,11 @@ public: BN_mpi2bn(pch, (int)(p - pch), this); } + int64_t getint64() + { + return (int64_t) getuint64(); + } + uint64_t getuint64() { size_t nSize = BN_bn2mpi(this, NULL); diff --git a/src/main.cpp b/src/main.cpp index 7cce81e..48de169 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -492,10 +492,10 @@ bool CTransaction::CheckTransaction() const return DoS(100, error("CTransaction::CheckTransaction() : size limits failed")); // Check for negative or overflow output values - int64_t nValueOut = 0; - for (unsigned int i = 0; i < vout.size(); i++) + CBigNum nValueOut = 0; + for (uint32_t i = 0; i < vout.size(); i++) { - const CTxOut& txout = vout[i]; + const auto& txout = vout[i]; if (txout.IsEmpty() && !IsCoinBase() && !IsCoinStake()) return DoS(100, error("CTransaction::CheckTransaction() : txout empty for user transaction")); if (!MoneyRange(txout.nValue)) @@ -1371,12 +1371,15 @@ int64_t CTransaction::GetValueIn(const MapPrevTx& inputs) const if (IsCoinBase()) return 0; - int64_t nResult = 0; - for (unsigned int i = 0; i < vin.size(); i++) + CBigNum nResult = 0; + for (uint32_t i = 0; i < vin.size(); i++) { - nResult += GetOutputFor(vin[i], inputs).nValue; + const auto& txOut = GetOutputFor(vin[i], inputs); + nResult += txOut.nValue; + if (!MoneyRange(txOut.nValue) || !MoneyRange(nResult)) + throw runtime_error("CTransaction::GetValueIn() : value out of range"); } - return nResult; + return nResult.getint64(); } @@ -1418,32 +1421,34 @@ bool CTransaction::ConnectInputs(CTxDB& txdb, MapPrevTx inputs, map 0); - CTxIndex& txindex = inputs[prevout.hash].first; - CTransaction& txPrev = inputs[prevout.hash].second; - - if (prevout.n >= txPrev.vout.size() || prevout.n >= txindex.vSpent.size()) - return DoS(100, error("ConnectInputs() : %s prevout.n out of range %d %" PRIszu " %" PRIszu " prev tx %s\n%s", GetHash().ToString().substr(0,10).c_str(), prevout.n, txPrev.vout.size(), txindex.vSpent.size(), prevout.hash.ToString().substr(0,10).c_str(), txPrev.ToString().c_str())); - - // If prev is coinbase or coinstake, check that it's matured - if (txPrev.IsCoinBase() || txPrev.IsCoinStake()) - for (const CBlockIndex* pindex = pindexBlock; pindex && pindexBlock->nHeight - pindex->nHeight < nCoinbaseMaturity; pindex = pindex->pprev) - if (pindex->nBlockPos == txindex.pos.nBlockPos && pindex->nFile == txindex.pos.nFile) - return error("ConnectInputs() : tried to spend %s at depth %d", txPrev.IsCoinBase() ? "coinbase" : "coinstake", pindexBlock->nHeight - pindex->nHeight); - - // ppcoin: check transaction timestamp - if (txPrev.nTime > nTime) - return DoS(100, error("ConnectInputs() : transaction timestamp earlier than input transaction")); - - // Check for negative or overflow input values - nValueIn += txPrev.vout[prevout.n].nValue; - if (!MoneyRange(txPrev.vout[prevout.n].nValue) || !MoneyRange(nValueIn)) - return DoS(100, error("ConnectInputs() : txin values out of range")); - + CBigNum bnValueIn = 0; + for (uint32_t i = 0; i < vin.size(); i++) + { + auto prevout = vin[i].prevout; + assert(inputs.count(prevout.hash) > 0); + CTxIndex& txindex = inputs[prevout.hash].first; + CTransaction& txPrev = inputs[prevout.hash].second; + + if (prevout.n >= txPrev.vout.size() || prevout.n >= txindex.vSpent.size()) + return DoS(100, error("ConnectInputs() : %s prevout.n out of range %d %" PRIszu " %" PRIszu " prev tx %s\n%s", GetHash().ToString().substr(0,10).c_str(), prevout.n, txPrev.vout.size(), txindex.vSpent.size(), prevout.hash.ToString().substr(0,10).c_str(), txPrev.ToString().c_str())); + + // If prev is coinbase or coinstake, check that it's matured + if (txPrev.IsCoinBase() || txPrev.IsCoinStake()) + for (const CBlockIndex* pindex = pindexBlock; pindex && pindexBlock->nHeight - pindex->nHeight < nCoinbaseMaturity; pindex = pindex->pprev) + if (pindex->nBlockPos == txindex.pos.nBlockPos && pindex->nFile == txindex.pos.nFile) + return error("ConnectInputs() : tried to spend %s at depth %d", txPrev.IsCoinBase() ? "coinbase" : "coinstake", pindexBlock->nHeight - pindex->nHeight); + + // ppcoin: check transaction timestamp + if (txPrev.nTime > nTime) + return DoS(100, error("ConnectInputs() : transaction timestamp earlier than input transaction")); + + // Check for negative or overflow input values + bnValueIn += txPrev.vout[prevout.n].nValue; + if (!MoneyRange(txPrev.vout[prevout.n].nValue) || !MoneyRange(bnValueIn)) + return DoS(100, error("ConnectInputs() : txin values out of range")); + } + nValueIn = bnValueIn.getint64(); } if (pvChecks) @@ -1525,11 +1530,7 @@ bool CTransaction::ConnectInputs(CTxDB& txdb, MapPrevTx inputs, map nValueIn) + if (GetValueOut() > bnValueIn.getint64()) return false; } diff --git a/src/main.h b/src/main.h index b8e6cb7..d42c6b4 100644 --- a/src/main.h +++ b/src/main.h @@ -53,6 +53,7 @@ static const int64_t MIN_TXOUT_AMOUNT = CENT/100; inline bool MoneyRange(int64_t nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); } +inline bool MoneyRange(CBigNum nValue) { return (nValue >= 0 && nValue <= MAX_MONEY); } // Threshold for nLockTime: below this value it is interpreted as block number, otherwise as UNIX timestamp. static const unsigned int LOCKTIME_THRESHOLD = 500000000; // Tue Nov 5 00:53:20 1985 UTC // Maximum number of script-checking threads allowed @@ -581,14 +582,14 @@ public: */ int64_t GetValueOut() const { - int64_t nValueOut = 0; - for(const CTxOut& txout : vout) + CBigNum nValueOut = 0; + for(const auto& txout : vout) { nValueOut += txout.nValue; if (!MoneyRange(txout.nValue) || !MoneyRange(nValueOut)) throw runtime_error("CTransaction::GetValueOut() : value out of range"); } - return nValueOut; + return nValueOut.getint64(); } /** Amount of bitcoins coming in to this transaction diff --git a/src/wallet.cpp b/src/wallet.cpp index 9d0f666..033c391 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -1032,38 +1032,41 @@ bool CWallet::IsFromMe(const CTransaction& tx) const int64_t CWallet::GetDebit(const CTransaction& tx, const isminefilter& filter) const { - int64_t nDebit = 0; + CBigNum nDebit = 0; for(const CTxIn& txin : tx.vin) { - nDebit += GetDebit(txin, filter); - if (!MoneyRange(nDebit)) + auto nValue = GetDebit(txin, filter); + nDebit += nValue; + if (!MoneyRange(nValue) || !MoneyRange(nDebit)) throw runtime_error("CWallet::GetDebit() : value out of range"); } - return nDebit; + return nDebit.getint64(); } int64_t CWallet::GetCredit(const CTransaction& tx, const isminefilter& filter) const { - int64_t nCredit = 0; + CBigNum nCredit = 0; for(const CTxOut& txout : tx.vout) { - nCredit += GetCredit(txout, filter); - if (!MoneyRange(nCredit)) + auto nValue = GetCredit(txout, filter); + nCredit += nValue; + if (!MoneyRange(nValue) || !MoneyRange(nCredit)) throw runtime_error("CWallet::GetCredit() : value out of range"); } - return nCredit; + return nCredit.getint64(); } int64_t CWallet::GetChange(const CTransaction& tx) const { - int64_t nChange = 0; + CBigNum nChange = 0; for(const CTxOut& txout : tx.vout) { - nChange += GetChange(txout); - if (!MoneyRange(nChange)) + int64_t nValue = GetChange(txout); + nChange += nValue; + if (!MoneyRange(nValue) || !MoneyRange(nChange)) throw runtime_error("CWallet::GetChange() : value out of range"); } - return nChange; + return nChange.getint64(); } int64_t CWalletTx::GetTxTime() const @@ -1254,22 +1257,23 @@ int64_t CWalletTx::GetAvailableCredit(bool fUseCache) const return nAvailableCreditCached; } - int64_t nCredit = 0; - for (unsigned int i = 0; i < vout.size(); i++) + CBigNum nCredit = 0; + for (uint32_t i = 0; i < vout.size(); i++) { if (!IsSpent(i)) { const CTxOut &txout = vout[i]; - nCredit += pwallet->GetCredit(txout, MINE_SPENDABLE); - if (!MoneyRange(nCredit)) + int64_t nValue = pwallet->GetCredit(txout, MINE_SPENDABLE); + nCredit += nValue; + if (!MoneyRange(nValue) || !MoneyRange(nCredit)) throw runtime_error("CWalletTx::GetAvailableCredit() : value out of range"); } } - nAvailableCreditCached = nCredit; + nAvailableCreditCached = nCredit.getint64(); fAvailableCreditCached = true; - return nCredit; + return nCredit.getint64(); } int64_t CWalletTx::GetAvailableWatchCredit(bool fUseCache) const @@ -1283,22 +1287,23 @@ int64_t CWalletTx::GetAvailableWatchCredit(bool fUseCache) const return nAvailableWatchCreditCached; } - int64_t nCredit = 0; + CBigNum nCredit = 0; for (unsigned int i = 0; i < vout.size(); i++) { if (!IsSpent(i)) { const CTxOut &txout = vout[i]; - nCredit += pwallet->GetCredit(txout, MINE_WATCH_ONLY); - if (!MoneyRange(nCredit)) + int64_t nValue = pwallet->GetCredit(txout, MINE_WATCH_ONLY); + nCredit += nValue; + if (!MoneyRange(nValue) || !MoneyRange(nCredit)) throw runtime_error("CWalletTx::GetAvailableCredit() : value out of range"); } } - nAvailableWatchCreditCached = nCredit; + nAvailableWatchCreditCached = nCredit.getint64(); fAvailableWatchCreditCached = true; - return nCredit; + return nCredit.getint64(); } int64_t CWalletTx::GetChange() const -- 1.7.1