From 7ff54e08aa42f10dae424711a0a3764a67eae295 Mon Sep 17 00:00:00 2001 From: "Ricardo M. Correia" Date: Mon, 14 May 2012 02:50:01 +0200 Subject: Don't overflow signed ints in CBigNum::setint64(). CBigNum::setint64() does 'n <<= 8', where n is of type "long long". This leads to shifting onto and past the sign bit, which is undefined behavior in C++11 and can cause problems in the future. --- src/bignum.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/bignum.h b/src/bignum.h index 4a3fb38b00..e203b26a05 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -121,16 +121,22 @@ public: return (n > std::numeric_limits::max() ? std::numeric_limits::min() : -(int)n); } - void setint64(int64 n) + void setint64(int64 sn) { - unsigned char pch[sizeof(n) + 6]; + unsigned char pch[sizeof(sn) + 6]; unsigned char* p = pch + 4; - bool fNegative = false; - if (n < (int64)0) + bool fNegative; + uint64 n; + + if (sn < (int64)0) { - n = -n; + n = -sn; fNegative = true; + } else { + n = sn; + fNegative = false; } + bool fLeadingZeroes = true; for (int i = 0; i < 8; i++) { -- cgit v1.2.3 From b0d9f41cd222de5d13d2b9cec27474009029d5ec Mon Sep 17 00:00:00 2001 From: "Ricardo M. Correia" Date: Thu, 7 Jun 2012 19:11:15 +0200 Subject: Don't overflow integer on 32-bit machines. This was causing test_bitcoin to abort on a 32-bit system likely due to -ftrapv. --- src/util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/util.h b/src/util.h index 284cf33c4b..dc72968326 100644 --- a/src/util.h +++ b/src/util.h @@ -396,7 +396,7 @@ inline int64 GetPerformanceCounter() #else timeval t; gettimeofday(&t, NULL); - nCounter = t.tv_sec * 1000000 + t.tv_usec; + nCounter = (int64) t.tv_sec * 1000000 + t.tv_usec; #endif return nCounter; } -- cgit v1.2.3 From c3def40293a89307d296ff66653b5927165c3c18 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 15 May 2012 15:53:30 -0400 Subject: Optimize orphan transaction handling Changes suggested by Sergio Demian Lerner to help prevent potential DoS attacks. --- src/main.cpp | 46 ++++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 0f45b2e16b..b7067b2092 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -45,7 +45,7 @@ map mapOrphanBlocks; multimap mapOrphanBlocksByPrev; map mapOrphanTransactions; -multimap mapOrphanTransactionsByPrev; +map > mapOrphanTransactionsByPrev; double dHashesPerSec; @@ -160,17 +160,37 @@ void static ResendWalletTransactions() // mapOrphanTransactions // -void AddOrphanTx(const CDataStream& vMsg) +bool AddOrphanTx(const CDataStream& vMsg) { CTransaction tx; CDataStream(vMsg) >> tx; uint256 hash = tx.GetHash(); if (mapOrphanTransactions.count(hash)) - return; + return false; + + CDataStream* pvMsg = new CDataStream(vMsg); + + // Ignore big transactions, to avoid a + // send-big-orphans memory exhaustion attack. If a peer has a legitimate + // large transaction with a missing parent then we assume + // it will rebroadcast it later, after the parent transaction(s) + // have been mined or received. + // 10,000 orphans, each of which is at most 5,000 bytes big is + // at most 500 megabytes of orphans: + if (pvMsg->size() > 5000) + { + delete pvMsg; + printf("ignoring large orphan tx (size: %u, hash: %s)\n", pvMsg->size(), hash.ToString().substr(0,10).c_str()); + return false; + } - CDataStream* pvMsg = mapOrphanTransactions[hash] = new CDataStream(vMsg); + mapOrphanTransactions[hash] = pvMsg; BOOST_FOREACH(const CTxIn& txin, tx.vin) - mapOrphanTransactionsByPrev.insert(make_pair(txin.prevout.hash, pvMsg)); + mapOrphanTransactionsByPrev[txin.prevout.hash].insert(make_pair(hash, pvMsg)); + + printf("stored orphan tx %s (mapsz %u)\n", hash.ToString().substr(0,10).c_str(), + mapOrphanTransactions.size()); + return true; } void static EraseOrphanTx(uint256 hash) @@ -182,14 +202,9 @@ void static EraseOrphanTx(uint256 hash) CDataStream(*pvMsg) >> tx; BOOST_FOREACH(const CTxIn& txin, tx.vin) { - for (multimap::iterator mi = mapOrphanTransactionsByPrev.lower_bound(txin.prevout.hash); - mi != mapOrphanTransactionsByPrev.upper_bound(txin.prevout.hash);) - { - if ((*mi).second == pvMsg) - mapOrphanTransactionsByPrev.erase(mi++); - else - mi++; - } + mapOrphanTransactionsByPrev[txin.prevout.hash].erase(hash); + if (mapOrphanTransactionsByPrev[txin.prevout.hash].empty()) + mapOrphanTransactionsByPrev.erase(txin.prevout.hash); } delete pvMsg; mapOrphanTransactions.erase(hash); @@ -2371,8 +2386,8 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) for (unsigned int i = 0; i < vWorkQueue.size(); i++) { uint256 hashPrev = vWorkQueue[i]; - for (multimap::iterator mi = mapOrphanTransactionsByPrev.lower_bound(hashPrev); - mi != mapOrphanTransactionsByPrev.upper_bound(hashPrev); + for (map::iterator mi = mapOrphanTransactionsByPrev[hashPrev].begin(); + mi != mapOrphanTransactionsByPrev[hashPrev].end(); ++mi) { const CDataStream& vMsg = *((*mi).second); @@ -2396,7 +2411,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) } else if (fMissingInputs) { - printf("storing orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); AddOrphanTx(vMsg); // DoS prevention: do not allow mapOrphanTransactions to grow unbounded -- cgit v1.2.3 From ce1a071f6d6d1548974796c4327399659415b489 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Wed, 16 May 2012 11:26:56 -0400 Subject: Further DoS prevention: Verify signatures last Loop over all inputs doing inexpensive validity checks first, and then loop over them a second time doing expensive signature checks. This helps prevent possible CPU exhaustion attacks where an attacker tries to make a victim waste time checking signatures for invalid transactions. --- src/main.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index b7067b2092..00f0633431 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1033,15 +1033,26 @@ bool CTransaction::ConnectInputs(MapPrevTx inputs, if (pindex->nBlockPos == txindex.pos.nBlockPos && pindex->nFile == txindex.pos.nFile) return error("ConnectInputs() : tried to spend coinbase at depth %d", pindexBlock->nHeight - pindex->nHeight); - // Check for conflicts (double-spend) - if (!txindex.vSpent[prevout.n].IsNull()) - return fMiner ? false : error("ConnectInputs() : %s prev tx already used at %s", GetHash().ToString().substr(0,10).c_str(), txindex.vSpent[prevout.n].ToString().c_str()); - // Check for negative or overflow input values nValueIn += txPrev.vout[prevout.n].nValue; if (!MoneyRange(txPrev.vout[prevout.n].nValue) || !MoneyRange(nValueIn)) return error("ConnectInputs() : txin values out of range"); + } + // The first loop above does all the inexpensive checks. + // Only if ALL inputs pass do we perform expensive ECDSA signature checks. + // Helps prevent CPU exhaustion attacks. + for (unsigned int i = 0; i < vin.size(); i++) + { + COutPoint prevout = vin[i].prevout; + assert(inputs.count(prevout.hash) > 0); + CTxIndex& txindex = inputs[prevout.hash].first; + CTransaction& txPrev = inputs[prevout.hash].second; + + // Check for conflicts (double-spend) + if (!txindex.vSpent[prevout.n].IsNull()) + return fMiner ? false : error("ConnectInputs() : %s prev tx already used at %s", GetHash().ToString().substr(0,10).c_str(), txindex.vSpent[prevout.n].ToString().c_str()); + // Verify signature if (!VerifySignature(txPrev, *this, i, fStrictPayToScriptHash, 0)) { -- cgit v1.2.3 From 01473c3f40cea8209186e737ae20289eebcb8898 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Thu, 17 May 2012 10:12:04 -0400 Subject: Remove invalid dependent orphans from memory Remove orphan transactions from memory once all of their parent transactions are received and they're still not valid. Thanks to Sergio Demian Lerner for suggesting this fix. --- src/main.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 00f0633431..432ec871f5 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2378,6 +2378,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) else if (strCommand == "tx") { vector vWorkQueue; + vector vEraseQueue; CDataStream vMsg(vRecv); CTransaction tx; vRecv >> tx; @@ -2392,6 +2393,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) RelayMessage(inv, vMsg); mapAlreadyAskedFor.erase(inv); vWorkQueue.push_back(inv.hash); + vEraseQueue.push_back(inv.hash); // Recursively process any orphan transactions that depended on this one for (unsigned int i = 0; i < vWorkQueue.size(); i++) @@ -2405,19 +2407,27 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) CTransaction tx; CDataStream(vMsg) >> tx; CInv inv(MSG_TX, tx.GetHash()); + bool fMissingInputs2 = false; - if (tx.AcceptToMemoryPool(true)) + if (tx.AcceptToMemoryPool(true, &fMissingInputs2)) { printf(" accepted orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); SyncWithWallets(tx, NULL, true); RelayMessage(inv, vMsg); mapAlreadyAskedFor.erase(inv); vWorkQueue.push_back(inv.hash); + vEraseQueue.push_back(inv.hash); + } + else if (!fMissingInputs2) + { + // invalid orphan + vEraseQueue.push_back(inv.hash); + printf(" removed invalid orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); } } } - BOOST_FOREACH(uint256 hash, vWorkQueue) + BOOST_FOREACH(uint256 hash, vEraseQueue) EraseOrphanTx(hash); } else if (fMissingInputs) -- cgit v1.2.3 From b199f7547f357711873327347c0f368248e99032 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 20 Jun 2012 17:59:36 +0000 Subject: Bump VERSION so we can differentiate between 0.4.7rc2 and 0.4.7rc3 --- src/serialize.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/serialize.h b/src/serialize.h index 8cdfb30b90..c7e64dac76 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -59,7 +59,7 @@ class CDataStream; class CAutoFile; static const unsigned int MAX_SIZE = 0x02000000; -static const int VERSION = 40701; +static const int VERSION = 40703; static const char* pszSubVer = ""; static const bool VERSION_IS_BETA = true; -- cgit v1.2.3 From 3023e782bdaee3448e1543b482cf5cd022c9699f Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 19 Jun 2012 15:50:12 -0400 Subject: print large orphan warning BEFORE deleting pvMsg --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/main.cpp b/src/main.cpp index 432ec871f5..7ce7c92e5d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -179,8 +179,8 @@ bool AddOrphanTx(const CDataStream& vMsg) // at most 500 megabytes of orphans: if (pvMsg->size() > 5000) { - delete pvMsg; printf("ignoring large orphan tx (size: %u, hash: %s)\n", pvMsg->size(), hash.ToString().substr(0,10).c_str()); + delete pvMsg; return false; } -- cgit v1.2.3