From f718aedd9f244ee77a40f182bf6c6737730d975c Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Thu, 17 May 2012 12:13:14 -0400 Subject: Refactor: GetRandHash() method for util --- src/main.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 427e435a9..d006510d1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -201,9 +201,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) while (mapOrphanTransactions.size() > nMaxOrphans) { // Evict a random orphan: - std::vector randbytes(32); - RAND_bytes(&randbytes[0], 32); - uint256 randomhash(randbytes); + uint256 randomhash = GetRandHash(); map::iterator it = mapOrphanTransactions.lower_bound(randomhash); if (it == mapOrphanTransactions.end()) it = mapOrphanTransactions.begin(); @@ -2354,7 +2352,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) // at a time so the setAddrKnowns of the chosen nodes prevent repeats static uint256 hashSalt; if (hashSalt == 0) - RAND_bytes((unsigned char*)&hashSalt, sizeof(hashSalt)); + hashSalt = GetRandHash(); int64 hashAddr = addr.GetHash(); uint256 hashRand = hashSalt ^ (hashAddr<<32) ^ ((GetTime()+hashAddr)/(24*60*60)); hashRand = Hash(BEGIN(hashRand), END(hashRand)); @@ -2945,7 +2943,7 @@ bool SendMessages(CNode* pto, bool fSendTrickle) // 1/4 of tx invs blast to all immediately static uint256 hashSalt; if (hashSalt == 0) - RAND_bytes((unsigned char*)&hashSalt, sizeof(hashSalt)); + hashSalt = GetRandHash(); uint256 hashRand = inv.hash ^ hashSalt; hashRand = Hash(BEGIN(hashRand), END(hashRand)); bool fTrickleWait = ((hashRand & 3) != 0); -- cgit v1.2.3 From 77b99cf7ad8f15f6228b07d5d01cb20c849519b8 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/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index d006510d1..59d8bc801 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -43,7 +43,7 @@ map mapOrphanBlocks; multimap mapOrphanBlocksByPrev; map mapOrphanTransactions; -multimap mapOrphanTransactionsByPrev; +map > mapOrphanTransactionsByPrev; // Constant stuff for coinbase transactions we create: CScript COINBASE_FLAGS; @@ -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); @@ -2569,8 +2584,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); @@ -2594,7 +2609,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 7a15109c043f8182cc996bf762f0a62c72646ff9 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/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 59d8bc801..a878d1e5b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2564,6 +2564,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) else if (strCommand == "tx") { vector vWorkQueue; + vector vEraseQueue; CDataStream vMsg(vRecv); CTxDB txdb("r"); CTransaction tx; @@ -2579,6 +2580,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++) @@ -2592,19 +2594,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(txdb, true)) + if (tx.AcceptToMemoryPool(txdb, 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 4add41a2a6fa1229277e00e01a8b4111e7cb21d6 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 | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index a878d1e5b..263f1e6fe 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1142,17 +1142,28 @@ 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 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")); + + } + // 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) // This doesn't trigger the DoS code on purpose; if it did, it would make it easier // for an attacker to attempt to split the network. 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 DoS(100, error("ConnectInputs() : txin values out of range")); - // Skip ECDSA signature verification when connecting blocks (fBlock=true) // before the last blockchain checkpoint. This is safe because block merkle hashes are // still computed and checked, and any change will be caught at the next checkpoint. -- cgit v1.2.3