From 77b99cf7ad8f15f6228b07d5d01cb20c849519b8 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 15 May 2012 15:53:30 -0400 Subject: [PATCH] Optimize orphan transaction handling Changes suggested by Sergio Demian Lerner to help prevent potential DoS attacks. --- src/main.cpp | 46 +++++++++++++++++++++++++++--------------- src/test/DoS_tests.cpp | 30 +++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index d006510d..59d8bc80 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 diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index 3a2b1d9d..c0d414d0 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -13,10 +13,10 @@ #include // Tests this internal-to-main.cpp method: -extern void AddOrphanTx(const CDataStream& vMsg); +extern bool AddOrphanTx(const CDataStream& vMsg); extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans); extern std::map mapOrphanTransactions; -extern std::multimap mapOrphanTransactionsByPrev; +extern std::map > mapOrphanTransactionsByPrev; CService ip(uint32_t i) { @@ -184,6 +184,32 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) AddOrphanTx(ds); } + // This really-big orphan should be ignored: + for (int i = 0; i < 10; i++) + { + CTransaction txPrev = RandomOrphan(); + + CTransaction tx; + tx.vout.resize(1); + tx.vout[0].nValue = 1*CENT; + tx.vout[0].scriptPubKey.SetBitcoinAddress(key.GetPubKey()); + tx.vin.resize(500); + for (int j = 0; j < tx.vin.size(); j++) + { + tx.vin[j].prevout.n = j; + tx.vin[j].prevout.hash = txPrev.GetHash(); + } + SignSignature(keystore, txPrev, tx, 0); + // Re-use same signature for other inputs + // (they don't have to be valid for this test) + for (int j = 1; j < tx.vin.size(); j++) + tx.vin[j].scriptSig = tx.vin[0].scriptSig; + + CDataStream ds(SER_DISK, CLIENT_VERSION); + ds << tx; + BOOST_CHECK(!AddOrphanTx(ds)); + } + // Test LimitOrphanTxSize() function: LimitOrphanTxSize(40); BOOST_CHECK(mapOrphanTransactions.size() <= 40);