Remove IsFromMe() check in CTxMemPool::accept()

Fixes issue #2178 : attacker could penny-flood with invalid-signature
transactions to deduce which addresses belonged to your node.

I'm committing this early for code review; I still need to write up
a test plan.

Executive summary of fix: check all transactions received from the network
for penny-flood rate-limiting before adding to the memory pool. But do NOT
ratelimit transactions added to the memory pool:
  - because of blockchain reorgs
  - stored in the wallet and added at startup
  - sent from the GUI or one of the send* RPC commands (CWallet::CommitTransaction)

The limit-free-transactions code really should be a method on CNode, with
counters per-peer. But that is a bigger change for another day.
This commit is contained in:
Gavin Andresen 2013-01-14 16:52:33 -05:00
parent c83c3cbe97
commit ce99358f4a
4 changed files with 31 additions and 32 deletions

View File

@ -627,7 +627,7 @@ void CTxMemPool::pruneSpent(const uint256 &hashTx, CCoins &coins)
} }
} }
bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs, bool fLimitFree,
bool* pfMissingInputs) bool* pfMissingInputs)
{ {
if (pfMissingInputs) if (pfMissingInputs)
@ -733,7 +733,7 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs,
// Don't accept it if it can't get into a block // Don't accept it if it can't get into a block
int64 txMinFee = tx.GetMinFee(1000, true, GMF_RELAY); int64 txMinFee = tx.GetMinFee(1000, true, GMF_RELAY);
if (nFees < txMinFee) if (fLimitFree && nFees < txMinFee)
return error("CTxMemPool::accept() : not enough fees %s, %"PRI64d" < %"PRI64d, return error("CTxMemPool::accept() : not enough fees %s, %"PRI64d" < %"PRI64d,
hash.ToString().c_str(), hash.ToString().c_str(),
nFees, txMinFee); nFees, txMinFee);
@ -741,25 +741,24 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs,
// Continuously rate-limit free transactions // Continuously rate-limit free transactions
// This mitigates 'penny-flooding' -- sending thousands of free transactions just to // This mitigates 'penny-flooding' -- sending thousands of free transactions just to
// be annoying or make others' transactions take longer to confirm. // be annoying or make others' transactions take longer to confirm.
if (nFees < MIN_RELAY_TX_FEE) if (fLimitFree && nFees < MIN_RELAY_TX_FEE)
{ {
static CCriticalSection cs;
static double dFreeCount; static double dFreeCount;
static int64 nLastTime; static int64 nLastTime;
int64 nNow = GetTime(); int64 nNow = GetTime();
{ LOCK(cs);
// Use an exponentially decaying ~10-minute window:
dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); // Use an exponentially decaying ~10-minute window:
nLastTime = nNow; dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime));
// -limitfreerelay unit is thousand-bytes-per-minute nLastTime = nNow;
// At default rate it would take over a month to fill 1GB // -limitfreerelay unit is thousand-bytes-per-minute
if (dFreeCount > GetArg("-limitfreerelay", 15)*10*1000 && !IsFromMe(tx)) // At default rate it would take over a month to fill 1GB
return error("CTxMemPool::accept() : free transaction rejected by rate limiter"); if (dFreeCount > GetArg("-limitfreerelay", 15)*10*1000)
if (fDebug) return error("CTxMemPool::accept() : free transaction rejected by rate limiter");
printf("Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize); if (fDebug)
dFreeCount += nSize; printf("Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize);
} dFreeCount += nSize;
} }
// Check against previous transactions // Check against previous transactions
@ -792,9 +791,9 @@ bool CTxMemPool::accept(CTransaction &tx, bool fCheckInputs,
return true; return true;
} }
bool CTransaction::AcceptToMemoryPool(bool fCheckInputs, bool* pfMissingInputs) bool CTransaction::AcceptToMemoryPool(bool fCheckInputs, bool fLimitFree, bool* pfMissingInputs)
{ {
return mempool.accept(*this, fCheckInputs, pfMissingInputs); return mempool.accept(*this, fCheckInputs, fLimitFree, pfMissingInputs);
} }
bool CTxMemPool::addUnchecked(const uint256& hash, CTransaction &tx) bool CTxMemPool::addUnchecked(const uint256& hash, CTransaction &tx)
@ -905,9 +904,9 @@ int CMerkleTx::GetBlocksToMaturity() const
} }
bool CMerkleTx::AcceptToMemoryPool(bool fCheckInputs) bool CMerkleTx::AcceptToMemoryPool(bool fCheckInputs, bool fLimitFree)
{ {
return CTransaction::AcceptToMemoryPool(fCheckInputs); return CTransaction::AcceptToMemoryPool(fCheckInputs, fLimitFree);
} }
@ -923,10 +922,10 @@ bool CWalletTx::AcceptWalletTransaction(bool fCheckInputs)
{ {
uint256 hash = tx.GetHash(); uint256 hash = tx.GetHash();
if (!mempool.exists(hash) && pcoinsTip->HaveCoins(hash)) if (!mempool.exists(hash) && pcoinsTip->HaveCoins(hash))
tx.AcceptToMemoryPool(fCheckInputs); tx.AcceptToMemoryPool(fCheckInputs, false);
} }
} }
return AcceptToMemoryPool(fCheckInputs); return AcceptToMemoryPool(fCheckInputs, false);
} }
return false; return false;
} }
@ -1797,7 +1796,7 @@ bool SetBestChain(CBlockIndex* pindexNew)
// Resurrect memory transactions that were in the disconnected branch // Resurrect memory transactions that were in the disconnected branch
BOOST_FOREACH(CTransaction& tx, vResurrect) BOOST_FOREACH(CTransaction& tx, vResurrect)
tx.AcceptToMemoryPool(); tx.AcceptToMemoryPool(true, false);
// Delete redundant memory transactions that are in the connected branch // Delete redundant memory transactions that are in the connected branch
BOOST_FOREACH(CTransaction& tx, vDelete) { BOOST_FOREACH(CTransaction& tx, vDelete) {
@ -3181,7 +3180,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
pfrom->AddInventoryKnown(inv); pfrom->AddInventoryKnown(inv);
bool fMissingInputs = false; bool fMissingInputs = false;
if (tx.AcceptToMemoryPool(true, &fMissingInputs)) if (tx.AcceptToMemoryPool(true, true, &fMissingInputs))
{ {
SyncWithWallets(inv.hash, tx, NULL, true); SyncWithWallets(inv.hash, tx, NULL, true);
RelayMessage(inv, vMsg); RelayMessage(inv, vMsg);
@ -3203,7 +3202,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
CInv inv(MSG_TX, tx.GetHash()); CInv inv(MSG_TX, tx.GetHash());
bool fMissingInputs2 = false; bool fMissingInputs2 = false;
if (tx.AcceptToMemoryPool(true, &fMissingInputs2)) if (tx.AcceptToMemoryPool(true, true, &fMissingInputs2))
{ {
printf(" accepted orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); printf(" accepted orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str());
SyncWithWallets(inv.hash, tx, NULL, true); SyncWithWallets(inv.hash, tx, NULL, true);
@ -3214,9 +3213,9 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
} }
else if (!fMissingInputs2) else if (!fMissingInputs2)
{ {
// invalid orphan // invalid or too-little-fee orphan
vEraseQueue.push_back(inv.hash); vEraseQueue.push_back(inv.hash);
printf(" removed invalid orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str()); printf(" removed orphan tx %s\n", inv.hash.ToString().substr(0,10).c_str());
} }
} }
} }

View File

@ -649,7 +649,7 @@ public:
bool CheckTransaction() const; bool CheckTransaction() const;
// Try to accept this transaction into the memory pool // Try to accept this transaction into the memory pool
bool AcceptToMemoryPool(bool fCheckInputs=true, bool* pfMissingInputs=NULL); bool AcceptToMemoryPool(bool fCheckInputs=true, bool fLimitFree = true, bool* pfMissingInputs=NULL);
protected: protected:
static const CTxOut &GetOutputFor(const CTxIn& input, CCoinsViewCache& mapInputs); static const CTxOut &GetOutputFor(const CTxIn& input, CCoinsViewCache& mapInputs);
@ -1103,7 +1103,7 @@ public:
int GetDepthInMainChain() const { CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet); } int GetDepthInMainChain() const { CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet); }
bool IsInMainChain() const { return GetDepthInMainChain() > 0; } bool IsInMainChain() const { return GetDepthInMainChain() > 0; }
int GetBlocksToMaturity() const; int GetBlocksToMaturity() const;
bool AcceptToMemoryPool(bool fCheckInputs=true); bool AcceptToMemoryPool(bool fCheckInputs=true, bool fLimitFree=true);
}; };
@ -1882,7 +1882,7 @@ public:
std::map<uint256, CTransaction> mapTx; std::map<uint256, CTransaction> mapTx;
std::map<COutPoint, CInPoint> mapNextTx; std::map<COutPoint, CInPoint> mapNextTx;
bool accept(CTransaction &tx, bool fCheckInputs, bool* pfMissingInputs); bool accept(CTransaction &tx, bool fCheckInputs, bool fLimitFree, bool* pfMissingInputs);
bool addUnchecked(const uint256& hash, CTransaction &tx); bool addUnchecked(const uint256& hash, 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);

View File

@ -546,7 +546,7 @@ Value sendrawtransaction(const Array& params, bool fHelp)
fHave = view.GetCoins(hashTx, existingCoins); fHave = view.GetCoins(hashTx, existingCoins);
if (!fHave) { if (!fHave) {
// push to local node // push to local node
if (!tx.AcceptToMemoryPool()) if (!tx.AcceptToMemoryPool(true, false))
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX rejected"); throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX rejected");
} }
} }

View File

@ -1279,7 +1279,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey)
mapRequestCount[wtxNew.GetHash()] = 0; mapRequestCount[wtxNew.GetHash()] = 0;
// Broadcast // Broadcast
if (!wtxNew.AcceptToMemoryPool()) if (!wtxNew.AcceptToMemoryPool(true, false))
{ {
// This must not fail. The transaction has already been signed and recorded. // This must not fail. The transaction has already been signed and recorded.
printf("CommitTransaction() : Error: Transaction not valid"); printf("CommitTransaction() : Error: Transaction not valid");