phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-12 04:42:36 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions

Simplify storage of orphan transactions

Browse source 
Orphan transactions were stored as a CDataStream pointer;
this changes the mapOrphanTransactions data structures to
store orphans as a CTransaction.

This also fixes CVE-2013-4627 by always re-serializing
transactions before relaying them.
This commit is contained in:
Gavin Andresen 2013-08-02 15:14:44 +10:00
parent 8f6f92c72b
commit 159bc48193
3 changed files with 39 additions and 60 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

66
src/main.cpp
View file

@ -59,8 +59,8 @@ CMedianFilter<int> cPeerBlockCounts(8, 0); // Amount of blocks that other nodes
map<uint256, CBlock*> mapOrphanBlocks; map<uint256, CBlock*> mapOrphanBlocks;
multimap<uint256, CBlock*> mapOrphanBlocksByPrev; multimap<uint256, CBlock*> mapOrphanBlocksByPrev;
map<uint256, CDataStream*> mapOrphanTransactions; map<uint256, CTransaction> mapOrphanTransactions;
map<uint256, map<uint256, CDataStream*> > mapOrphanTransactionsByPrev; map<uint256, set<uint256> > mapOrphanTransactionsByPrev;
// Constant stuff for coinbase transactions we create: // Constant stuff for coinbase transactions we create:
CScript COINBASE_FLAGS; CScript COINBASE_FLAGS;
@ -399,16 +399,12 @@ CBlockTreeDB *pblocktree = NULL;
// mapOrphanTransactions // mapOrphanTransactions
// //
bool AddOrphanTx(const CDataStream& vMsg) bool AddOrphanTx(const CTransaction& tx)
{ {
CTransaction tx;
CDataStream(vMsg) >> tx;
uint256 hash = tx.GetHash(); uint256 hash = tx.GetHash();
if (mapOrphanTransactions.count(hash)) if (mapOrphanTransactions.count(hash))
return false; return false;
CDataStream* pvMsg = new CDataStream(vMsg);
// Ignore big transactions, to avoid a // Ignore big transactions, to avoid a
// send-big-orphans memory exhaustion attack. If a peer has a legitimate // send-big-orphans memory exhaustion attack. If a peer has a legitimate
// large transaction with a missing parent then we assume // large transaction with a missing parent then we assume
@ -416,16 +412,16 @@ bool AddOrphanTx(const CDataStream& vMsg)
// have been mined or received. // have been mined or received.
// 10,000 orphans, each of which is at most 5,000 bytes big is // 10,000 orphans, each of which is at most 5,000 bytes big is
// at most 500 megabytes of orphans: // at most 500 megabytes of orphans:
if (pvMsg->size() > 5000) unsigned int sz = tx.GetSerializeSize(SER_NETWORK, CTransaction::CURRENT_VERSION);
if (sz > 5000)
{ {
printf("ignoring large orphan tx (size: %"PRIszu", hash: %s)\n", pvMsg->size(), hash.ToString().c_str()); printf("ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString().c_str());
delete pvMsg;
return false; return false;
} }
mapOrphanTransactions[hash] = pvMsg; mapOrphanTransactions[hash] = tx;
BOOST_FOREACH(const CTxIn& txin, tx.vin) BOOST_FOREACH(const CTxIn& txin, tx.vin)
mapOrphanTransactionsByPrev[txin.prevout.hash].insert(make_pair(hash, pvMsg)); mapOrphanTransactionsByPrev[txin.prevout.hash].insert(hash);
printf("stored orphan tx %s (mapsz %"PRIszu")\n", hash.ToString().c_str(), printf("stored orphan tx %s (mapsz %"PRIszu")\n", hash.ToString().c_str(),
mapOrphanTransactions.size()); mapOrphanTransactions.size());
@ -436,16 +432,13 @@ void static EraseOrphanTx(uint256 hash)
{ {
if (!mapOrphanTransactions.count(hash)) if (!mapOrphanTransactions.count(hash))
return; return;
const CDataStream* pvMsg = mapOrphanTransactions[hash]; const CTransaction& tx = mapOrphanTransactions[hash];
CTransaction tx;
CDataStream(*pvMsg) >> tx;
BOOST_FOREACH(const CTxIn& txin, tx.vin) BOOST_FOREACH(const CTxIn& txin, tx.vin)
{ {
mapOrphanTransactionsByPrev[txin.prevout.hash].erase(hash); mapOrphanTransactionsByPrev[txin.prevout.hash].erase(hash);
if (mapOrphanTransactionsByPrev[txin.prevout.hash].empty()) if (mapOrphanTransactionsByPrev[txin.prevout.hash].empty())
mapOrphanTransactionsByPrev.erase(txin.prevout.hash); mapOrphanTransactionsByPrev.erase(txin.prevout.hash);
} }
delete pvMsg;
mapOrphanTransactions.erase(hash); mapOrphanTransactions.erase(hash);
} }
@ -456,7 +449,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
{ {
// Evict a random orphan: // Evict a random orphan:
uint256 randomhash = GetRandHash(); uint256 randomhash = GetRandHash();
map<uint256, CDataStream*>::iterator it = mapOrphanTransactions.lower_bound(randomhash); map<uint256, CTransaction>::iterator it = mapOrphanTransactions.lower_bound(randomhash);
if (it == mapOrphanTransactions.end()) if (it == mapOrphanTransactions.end())
it = mapOrphanTransactions.begin(); it = mapOrphanTransactions.begin();
EraseOrphanTx(it->first); EraseOrphanTx(it->first);
@ -793,7 +786,7 @@ void CTxMemPool::pruneSpent(const uint256 &hashTx, CCoins &coins)
} }
} }
bool CTxMemPool::accept(CValidationState &state, CTransaction &tx, bool fLimitFree, bool CTxMemPool::accept(CValidationState &state, const CTransaction &tx, bool fLimitFree,
bool* pfMissingInputs) bool* pfMissingInputs)
{ {
if (pfMissingInputs) if (pfMissingInputs)
@ -960,7 +953,7 @@ bool CTxMemPool::accept(CValidationState &state, CTransaction &tx, bool fLimitFr
} }
bool CTxMemPool::addUnchecked(const uint256& hash, CTransaction &tx) bool CTxMemPool::addUnchecked(const uint256& hash, const CTransaction &tx)
{ {
// Add to memory pool without checking anything. Don't call this directly, // Add to memory pool without checking anything. Don't call this directly,
// call CTxMemPool::accept to properly check the transaction first. // call CTxMemPool::accept to properly check the transaction first.
@ -3602,7 +3595,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
CValidationState state; CValidationState state;
if (mempool.accept(state, tx, true, &fMissingInputs)) if (mempool.accept(state, tx, true, &fMissingInputs))
{ {
RelayTransaction(tx, inv.hash, vMsg); RelayTransaction(tx, inv.hash);
mapAlreadyAskedFor.erase(inv); mapAlreadyAskedFor.erase(inv);
vWorkQueue.push_back(inv.hash); vWorkQueue.push_back(inv.hash);
vEraseQueue.push_back(inv.hash); vEraseQueue.push_back(inv.hash);
@ -3611,31 +3604,31 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
for (unsigned int i = 0; i < vWorkQueue.size(); i++) for (unsigned int i = 0; i < vWorkQueue.size(); i++)
{ {
uint256 hashPrev = vWorkQueue[i]; uint256 hashPrev = vWorkQueue[i];
for (map<uint256, CDataStream*>::iterator mi = mapOrphanTransactionsByPrev[hashPrev].begin(); for (set<uint256>::iterator mi = mapOrphanTransactionsByPrev[hashPrev].begin();
mi != mapOrphanTransactionsByPrev[hashPrev].end(); mi != mapOrphanTransactionsByPrev[hashPrev].end();
++mi) ++mi)
{ {
const CDataStream& vMsg = *((*mi).second); const uint256& orphanHash = *mi;
CTransaction tx; const CTransaction& orphanTx = mapOrphanTransactions[orphanHash];
CDataStream(vMsg) >> tx;
CInv inv(MSG_TX, tx.GetHash());
bool fMissingInputs2 = false; bool fMissingInputs2 = false;
// Use a dummy CValidationState so someone can't setup nodes to counter-DoS based on orphan resolution (that is, feeding people an invalid transaction based on LegitTxX in order to get anyone relaying LegitTxX banned) // Use a dummy CValidationState so someone can't setup nodes to counter-DoS based on orphan
// resolution (that is, feeding people an invalid transaction based on LegitTxX in order to get
// anyone relaying LegitTxX banned)
CValidationState stateDummy; CValidationState stateDummy;
if (mempool.accept(stateDummy, tx, true, &fMissingInputs2)) if (mempool.accept(stateDummy, orphanTx, true, &fMissingInputs2))
{ {
printf(" accepted orphan tx %s\n", inv.hash.ToString().c_str()); printf(" accepted orphan tx %s\n", orphanHash.ToString().c_str());
RelayTransaction(tx, inv.hash, vMsg); RelayTransaction(orphanTx, orphanHash);
mapAlreadyAskedFor.erase(inv); mapAlreadyAskedFor.erase(CInv(MSG_TX, orphanHash));
vWorkQueue.push_back(inv.hash); vWorkQueue.push_back(orphanHash);
vEraseQueue.push_back(inv.hash); vEraseQueue.push_back(orphanHash);
} }
else if (!fMissingInputs2) else if (!fMissingInputs2)
{ {
// invalid or too-little-fee orphan // invalid or too-little-fee orphan
vEraseQueue.push_back(inv.hash); vEraseQueue.push_back(orphanHash);
printf(" removed orphan tx %s\n", inv.hash.ToString().c_str()); printf(" removed orphan tx %s\n", orphanHash.ToString().c_str());
} }
} }
} }
@ -3645,7 +3638,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
} }
else if (fMissingInputs) else if (fMissingInputs)
{ {
AddOrphanTx(vMsg); AddOrphanTx(tx);
// DoS prevention: do not allow mapOrphanTransactions to grow unbounded // DoS prevention: do not allow mapOrphanTransactions to grow unbounded
unsigned int nEvicted = LimitOrphanTxSize(MAX_ORPHAN_TRANSACTIONS); unsigned int nEvicted = LimitOrphanTxSize(MAX_ORPHAN_TRANSACTIONS);
@ -4132,9 +4125,6 @@ public:
mapOrphanBlocks.clear(); mapOrphanBlocks.clear();
// orphan transactions // orphan transactions
std::map<uint256, CDataStream*>::iterator it3 = mapOrphanTransactions.begin();
for (; it3 != mapOrphanTransactions.end(); it3++)
delete (*it3).second;
mapOrphanTransactions.clear(); mapOrphanTransactions.clear();
} }
} instance_of_cmaincleanup; } instance_of_cmaincleanup;

4
src/main.h
View file

@ -1081,8 +1081,8 @@ public:
std::map<uint256, CTransaction> mapTx; std::map<uint256, CTransaction> mapTx;
std::map<COutPoint, CInPoint> mapNextTx; std::map<COutPoint, CInPoint> mapNextTx;
bool accept(CValidationState &state, CTransaction &tx, bool fLimitFree, bool* pfMissingInputs); bool accept(CValidationState &state, const CTransaction &tx, bool fLimitFree, bool* pfMissingInputs);
bool addUnchecked(const uint256& hash, CTransaction &tx); bool addUnchecked(const uint256& hash, const CTransaction &tx);
bool remove(const CTransaction &tx, bool fRecursive = false); bool remove(const CTransaction &tx, bool fRecursive = false);
bool removeConflicts(const CTransaction &tx); bool removeConflicts(const CTransaction &tx);
void clear(); void clear();

29
src/test/DoS_tests.cpp
View file

@ -17,10 +17,10 @@
#include <stdint.h> #include <stdint.h>
// Tests this internal-to-main.cpp method: // Tests this internal-to-main.cpp method:
extern bool AddOrphanTx(const CDataStream& vMsg); extern bool AddOrphanTx(const CTransaction& tx);
extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans); extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
extern std::map<uint256, CDataStream*> mapOrphanTransactions; extern std::map<uint256, CTransaction> mapOrphanTransactions;
extern std::map<uint256, std::map<uint256, CDataStream*> > mapOrphanTransactionsByPrev; extern std::map<uint256, std::set<uint256> > mapOrphanTransactionsByPrev;
CService ip(uint32_t i) CService ip(uint32_t i)
{ {
@ -134,14 +134,11 @@ BOOST_AUTO_TEST_CASE(DoS_checknbits)
CTransaction RandomOrphan() CTransaction RandomOrphan()
{ {
std::map<uint256, CDataStream*>::iterator it; std::map<uint256, CTransaction>::iterator it;
it = mapOrphanTransactions.lower_bound(GetRandHash()); it = mapOrphanTransactions.lower_bound(GetRandHash());
if (it == mapOrphanTransactions.end()) if (it == mapOrphanTransactions.end())
it = mapOrphanTransactions.begin(); it = mapOrphanTransactions.begin();
const CDataStream* pvMsg = it->second; return it->second;
CTransaction tx;
CDataStream(*pvMsg) >> tx;
return tx;
} }
BOOST_AUTO_TEST_CASE(DoS_mapOrphans) BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
@ -163,9 +160,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
tx.vout[0].nValue = 1*CENT; tx.vout[0].nValue = 1*CENT;
tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID()); tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID());
CDataStream ds(SER_DISK, CLIENT_VERSION); AddOrphanTx(tx);
ds << tx;
AddOrphanTx(ds);
} }
// ... and 50 that depend on other orphans: // ... and 50 that depend on other orphans:
@ -182,9 +177,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID()); tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID());
SignSignature(keystore, txPrev, tx, 0); SignSignature(keystore, txPrev, tx, 0);
CDataStream ds(SER_DISK, CLIENT_VERSION); AddOrphanTx(tx);
ds << tx;
AddOrphanTx(ds);
} }
// This really-big orphan should be ignored: // This really-big orphan should be ignored:
@ -208,9 +201,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
for (unsigned int j = 1; j < tx.vin.size(); j++) for (unsigned int j = 1; j < tx.vin.size(); j++)
tx.vin[j].scriptSig = tx.vin[0].scriptSig; tx.vin[j].scriptSig = tx.vin[0].scriptSig;
CDataStream ds(SER_DISK, CLIENT_VERSION); BOOST_CHECK(!AddOrphanTx(tx));
ds << tx;
BOOST_CHECK(!AddOrphanTx(ds));
} }
// Test LimitOrphanTxSize() function: // Test LimitOrphanTxSize() function:
@ -247,9 +238,7 @@ BOOST_AUTO_TEST_CASE(DoS_checkSig)
tx.vout[0].nValue = 1*CENT; tx.vout[0].nValue = 1*CENT;
tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID()); tx.vout[0].scriptPubKey.SetDestination(key.GetPubKey().GetID());
CDataStream ds(SER_DISK, CLIENT_VERSION); AddOrphanTx(tx);
ds << tx;
AddOrphanTx(ds);
} }
// Create a transaction that depends on orphans: // Create a transaction that depends on orphans: