From 11766bec0ea465da263f78a6f473ca792da59280 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 19 Aug 2026 18:17:06 +0000 Subject: net: stop serving the 2009 marketplace to anonymous peers Four inherited handlers took attacker-controlled input and wrote to disk with nothing authorising it. None of them had a caller left anywhere in the tree, so all four were reachable only from the outside. checkorder called GenerateNewKey() -- AddKey(), a wallet.dat write -- for any peer that asked, and handed back the pubkey. Its comment claims the cache keeps one key per ip, and on a 2009 network of routable addresses it did. Not here: inbound peers arrive through a rendezvous tunnel and are tagged by BtfMarkerAddr() over three random bytes, so addr.ip is fresh every connection and the cache never hits. Reconnect in a loop and the node mints a key per connection, none of them in any backup taken beforehand, which for a pre-BIP32 wallet is how coins go missing. The map holding them was static and never trimmed. submitorder AddToWallet()'d a CWalletTx from any peer that sent one. It had to be a valid transaction; it did not have to be ours. review checked a signature the sender made with its own key, which proves possession and authorises nothing, then appended to reviews.dat by reading the whole vector, pushing one entry and writing it all back -- quadratic disk on an unbounded map off the socket -- then walked a link graph the same peer supplied and relayed the message to every other node. reply dispatched into CRequestTracker. Nothing ever called PushRequest, so the table was permanently empty and the handler was a parser and nothing else. Removed with the subsystem behind them: market.cpp, CProduct, CReview, CUser, CReviewDB, the advert publish templates and the subscribe channel machinery, none of which had an inbound handler either. MSG_REVIEW, MSG_PRODUCT and MSG_TABLE stay in the inv enum, unserved: CInv puts type on the wire as an integer and MSG_TX and MSG_BLOCK are shared with every other node. Unknown commands are already ignored for extensibility, so peers still running these lose nothing but an answer they never asked for. Verified by a clean build of bitflash-node. (cherry picked from commit ad9c393866c007a115f32d70a9017ce91066a7d1) --- src/net.cpp | 109 ++++-------------------------------------------------------- 1 file changed, 7 insertions(+), 102 deletions(-) (limited to 'src/net.cpp') diff --git a/src/net.cpp b/src/net.cpp index 72a7fda..e08f6f7 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -444,28 +444,9 @@ bool GetMyExternalIP(unsigned int& ipRet) -void AbandonRequests(void (*fn)(void*, CDataStream&), void* param1) -{ - // If the dialog might get closed before the reply comes back, - // call this in the destructor so it doesn't get called after it's deleted. - CRITICAL_BLOCK(cs_vNodes) - { - foreach(CNode* pnode, vNodes) - { - CRITICAL_BLOCK(pnode->cs_mapRequests) - { - for (map::iterator mi = pnode->mapRequests.begin(); mi != pnode->mapRequests.end();) - { - CRequestTracker& tracker = (*mi).second; - if (tracker.fn == fn && tracker.param1 == param1) - pnode->mapRequests.erase(mi++); - else - mi++; - } - } - } - } -} +// AbandonRequests() lived here, sweeping CNode::mapRequests for a callback a +// closing dialog had registered. Nothing ever registered one and nothing ever +// called this, so it went with CRequestTracker. @@ -473,25 +454,10 @@ void AbandonRequests(void (*fn)(void*, CDataStream&), void* param1) -// -// Subscription methods for the broadcast and subscription system. -// Channel numbers are message numbers, i.e. MSG_TABLE and MSG_PRODUCT. -// -// The subscription system uses a meet-in-the-middle strategy. -// With 100,000 nodes, if senders broadcast to 1000 random nodes and receivers -// subscribe to 1000 random nodes, 99.995% (1 - 0.99^1000) of messages will get through. -// - -bool AnySubscribed(unsigned int nChannel) -{ - if (pnodeLocalHost->IsSubscribed(nChannel)) - return true; - CRITICAL_BLOCK(cs_vNodes) - foreach(CNode* pnode, vNodes) - if (pnode->IsSubscribed(nChannel)) - return true; - return false; -} +// The broadcast/subscribe channel system (AnySubscribed, CNode::IsSubscribed, +// Subscribe, CancelSubscribe) stood here. Its channels were MSG_TABLE and +// MSG_PRODUCT, there was never an inbound handler for "subscribe", +// "sub-cancel", "publish" or "pub-cancel", and the objects it carried are gone. void CNode::PushGetBlocks(CBlockIndex* pindexBegin, uint256 hashEnd) { @@ -504,55 +470,6 @@ void CNode::PushGetBlocks(CBlockIndex* pindexBegin, uint256 hashEnd) PushMessage("getblocks", CBlockLocator(pindexBegin), hashEnd); } -bool CNode::IsSubscribed(unsigned int nChannel) -{ - if (nChannel >= vfSubscribe.size()) - return false; - return vfSubscribe[nChannel]; -} - -void CNode::Subscribe(unsigned int nChannel, unsigned int nHops) -{ - if (nChannel >= vfSubscribe.size()) - return; - - if (!AnySubscribed(nChannel)) - { - // Relay subscribe - CRITICAL_BLOCK(cs_vNodes) - foreach(CNode* pnode, vNodes) - if (pnode != this) - pnode->PushMessage("subscribe", nChannel, nHops); - } - - vfSubscribe[nChannel] = true; -} - -void CNode::CancelSubscribe(unsigned int nChannel) -{ - if (nChannel >= vfSubscribe.size()) - return; - - // Prevent from relaying cancel if wasn't subscribed - if (!vfSubscribe[nChannel]) - return; - vfSubscribe[nChannel] = false; - - if (!AnySubscribed(nChannel)) - { - // Relay subscription cancel - CRITICAL_BLOCK(cs_vNodes) - foreach(CNode* pnode, vNodes) - if (pnode != this) - pnode->PushMessage("sub-cancel", nChannel); - - // Clear memory, no longer subscribed - if (nChannel == MSG_PRODUCT) - CRITICAL_BLOCK(cs_mapProducts) - mapProducts.clear(); - } -} - @@ -1215,17 +1132,6 @@ void CNode::Disconnect() BtfCloseSocket(hSocket); hSocket = INVALID_SOCKET; - // All of a nodes broadcasts and subscriptions are automatically torn down - // when it goes down, so a node has to stay up to keep its broadcast going. - - CRITICAL_BLOCK(cs_mapProducts) - for (map::iterator mi = mapProducts.begin(); mi != mapProducts.end();) - AdvertRemoveSource(this, MSG_PRODUCT, 0, (*(mi++)).second); - - // Cancel subscriptions - for (unsigned int nChannel = 0; nChannel < vfSubscribe.size(); nChannel++) - if (vfSubscribe[nChannel]) - CancelSubscribe(nChannel); } @@ -1350,7 +1256,6 @@ void ThreadSocketHandler2(void* parg) bool fDelete = false; TRY_CRITICAL_BLOCK(pnode->cs_vSend) TRY_CRITICAL_BLOCK(pnode->cs_vRecv) - TRY_CRITICAL_BLOCK(pnode->cs_mapRequests) TRY_CRITICAL_BLOCK(pnode->cs_inventory) fDelete = true; if (fDelete) -- cgit v1.2.3