Do not use mempool for GETDATA for tx accepted after the last mempool req.

gmaxwell · gmaxwell · commit 7e908c7b826c · 2016-05-25T18:05:58.000Z
The ability to GETDATA a transaction which has not (yet) been relayed
 is a privacy loss vector.

The use of the mempool for this was added as part of the mempool p2p
 message and is only needed to fetch transactions returned by it.
diff --git a/src/main.cpp b/src/main.cpp
@@ -4503,7 +4503,10 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
                 }
                 if (!pushed && inv.type == MSG_TX) {
                     CTransaction tx;
-                    if (mempool.lookup(inv.hash, tx)) {
+                    int64_t txtime;
+                    // To protect privacy, do not answer getdata using the mempool when
+                    // that TX couldn't have been INVed in reply to a MEMPOOL request.
+                    if (mempool.lookup(inv.hash, tx, txtime) && txtime <= pfrom->timeLastMempoolReq) {
                         pfrom->PushMessage(NetMsgType::TX, tx);
                         pushed = true;
                     }
@@ -5902,6 +5905,7 @@ bool SendMessages(CNode* pto)
                         vInv.clear();
                     }
                 }
+                pto->timeLastMempoolReq = GetTime();
             }
 
             // Determine transactions to relay
diff --git a/src/net.cpp b/src/net.cpp
@@ -2396,6 +2396,7 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa
     fRelayTxes = false;
     fSentAddr = false;
     pfilter = new CBloomFilter();
+    timeLastMempoolReq = 0;
     nPingNonceSent = 0;
     nPingUsecStart = 0;
     nPingUsecTime = 0;
diff --git a/src/net.h b/src/net.h
@@ -17,6 +17,7 @@
 #include "sync.h"
 #include "uint256.h"
 
+#include <atomic>
 #include <deque>
 #include <stdint.h>
 
@@ -413,6 +414,8 @@ class CNode
     // Used for BIP35 mempool sending, also protected by cs_inventory
     bool fSendMempool;
 
+    // Last time a "MEMPOOL" request was serviced.
+    std::atomic<int64_t> timeLastMempoolReq;
     // Ping time measurement:
     // The pong reply we're expecting, or 0 if no pong expected.
     uint64_t nPingNonceSent;
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -789,15 +789,23 @@ void CTxMemPool::queryHashes(vector<uint256>& vtxid)
     std::sort(vtxid.begin(), vtxid.end(), DepthAndScoreComparator(this));
 }
 
-bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
+
+bool CTxMemPool::lookup(uint256 hash, CTransaction& result, int64_t& time) const
 {
     LOCK(cs);
     indexed_transaction_set::const_iterator i = mapTx.find(hash);
     if (i == mapTx.end()) return false;
     result = i->GetTx();
+    time = i->GetTime();
     return true;
 }
 
+bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
+{
+    int64_t time;
+    return CTxMemPool::lookup(hash, result, time);
+}
+
 bool CTxMemPool::lookupFeeRate(const uint256& hash, CFeeRate& feeRate) const
 {
     LOCK(cs);
diff --git a/src/txmempool.h b/src/txmempool.h
@@ -602,6 +602,7 @@ class CTxMemPool
     }
 
     bool lookup(uint256 hash, CTransaction& result) const;
+    bool lookup(uint256 hash, CTransaction& result, int64_t& time) const;
     bool lookupFeeRate(const uint256& hash, CFeeRate& feeRate) const;
 
     /** Estimate fee rate needed to get into the next nBlocks

Original file line number	Diff line number	Diff line change
`@@ -4503,7 +4503,10 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam`
`4503`	`4503`	`}`
`4504`	`4504`	`if (!pushed && inv.type == MSG_TX) {`
`4505`	`4505`	`CTransaction tx;`
`4506`		`- if (mempool.lookup(inv.hash, tx)) {`
	`4506`	`+ int64_t txtime;`
	`4507`	`+ // To protect privacy, do not answer getdata using the mempool when`
	`4508`	`+ // that TX couldn't have been INVed in reply to a MEMPOOL request.`
	`4509`	`+ if (mempool.lookup(inv.hash, tx, txtime) && txtime <= pfrom->timeLastMempoolReq) {`
`4507`	`4510`	`pfrom->PushMessage(NetMsgType::TX, tx);`
`4508`	`4511`	`pushed = true;`
`4509`	`4512`	`}`
`@@ -5902,6 +5905,7 @@ bool SendMessages(CNode* pto)`
`5902`	`5905`	`vInv.clear();`
`5903`	`5906`	`}`
`5904`	`5907`	`}`
	`5908`	`+ pto->timeLastMempoolReq = GetTime();`
`5905`	`5909`	`}`
`5906`	`5910`
`5907`	`5911`	`// Determine transactions to relay`
Original file line number	Diff line number	Diff line change
`@@ -789,15 +789,23 @@ void CTxMemPool::queryHashes(vector<uint256>& vtxid)`
`789`	`789`	`std::sort(vtxid.begin(), vtxid.end(), DepthAndScoreComparator(this));`
`790`	`790`	`}`
`791`	`791`
`792`		`-bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const`
	`792`	`+`
	`793`	`+bool CTxMemPool::lookup(uint256 hash, CTransaction& result, int64_t& time) const`
`793`	`794`	`{`
`794`	`795`	`LOCK(cs);`
`795`	`796`	`indexed_transaction_set::const_iterator i = mapTx.find(hash);`
`796`	`797`	`if (i == mapTx.end()) return false;`
`797`	`798`	`result = i->GetTx();`
	`799`	`+ time = i->GetTime();`
`798`	`800`	`return true;`
`799`	`801`	`}`
`800`	`802`
	`803`	`+bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const`
	`804`	`+{`
	`805`	`+ int64_t time;`
	`806`	`+ return CTxMemPool::lookup(hash, result, time);`
	`807`	`+}`
	`808`	`+`
`801`	`809`	`bool CTxMemPool::lookupFeeRate(const uint256& hash, CFeeRate& feeRate) const`
`802`	`810`	`{`
`803`	`811`	`LOCK(cs);`
Original file line number	Diff line number	Diff line change
`@@ -602,6 +602,7 @@ class CTxMemPool`
`602`	`602`	`}`
`603`	`603`
`604`	`604`	`bool lookup(uint256 hash, CTransaction& result) const;`
	`605`	`+ bool lookup(uint256 hash, CTransaction& result, int64_t& time) const;`
`605`	`606`	`bool lookupFeeRate(const uint256& hash, CFeeRate& feeRate) const;`
`606`	`607`
`607`	`608`	`/** Estimate fee rate needed to get into the next nBlocks`