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.h | 138 ++++++++------------------------------------------------------ 1 file changed, 17 insertions(+), 121 deletions(-) (limited to 'src/net.h') diff --git a/src/net.h b/src/net.h index 0317676..8b0245a 100644 --- a/src/net.h +++ b/src/net.h @@ -5,7 +5,6 @@ class CMessageHeader; class CAddress; class CInv; -class CRequestTracker; class CNode; // Defined in main.h, which is included after this file. Only ever used here as // a pointer, so the forward declaration is enough. @@ -14,7 +13,6 @@ class CBlockIndex; static const unsigned short DEFAULT_PORT = htons(8433); -static const unsigned int PUBLISH_HOPS = 5; enum { NODE_NETWORK = (1 << 0), @@ -214,8 +212,6 @@ void BtfChurnNoteHandshakeTimeout(const string& strBtfAddr, const string& strMee void BtfPexCollect(std::vector& vDescOut); // Verify descriptors a peer sent and remember the good ones. Returns how many. int BtfPexAccept(const std::vector& vDesc); -void AbandonRequests(void (*fn)(void*, CDataStream&), void* param1); -bool AnySubscribed(unsigned int nChannel); void ThreadBitcoinMiner(void* parg); // Starts MinerThreadCount() miners; returns how many came up. int StartMinerThreads(); @@ -489,13 +485,19 @@ public: +// MSG_REVIEW, MSG_PRODUCT and MSG_TABLE no longer have a handler, a producer +// or a store -- the marketplace they belonged to is gone. They stay in the +// enum, and their names stay in ppszTypeName below, because CInv serialises +// `type` as a plain integer: dropping them would renumber nothing today but +// would make it tempting to renumber later, and MSG_TX and MSG_BLOCK are wire +// values shared with every other node. Reserved, never emitted, never served. enum { MSG_TX = 1, MSG_BLOCK, - MSG_REVIEW, - MSG_PRODUCT, - MSG_TABLE, + MSG_REVIEW, // reserved, dead + MSG_PRODUCT, // reserved, dead + MSG_TABLE, // reserved, dead }; static const char* ppszTypeName[] = @@ -582,23 +584,9 @@ public: -class CRequestTracker -{ -public: - void (*fn)(void*, CDataStream&); - void* param1; - - explicit CRequestTracker(void (*fnIn)(void*, CDataStream&)=NULL, void* param1In=NULL) - { - fn = fnIn; - param1 = param1In; - } - - bool IsNull() - { - return fn == NULL; - } -}; +// CRequestTracker and the PushRequest/"reply" round trip lived here. Nothing +// in this tree ever called PushRequest, so the table was always empty and the +// "reply" handler was pure inbound parser. Both removed. @@ -644,9 +632,6 @@ protected: public: int64 nReleaseTime; int64 nDisconnectSince; - map mapRequests; - CCriticalSection cs_mapRequests; - // inventory based relay set setInventoryKnown; set setInventoryKnown2; @@ -654,9 +639,6 @@ public: CCriticalSection cs_inventory; multimap mapAskFor; - // publish and subscription - vector vfSubscribe; - // Last "btfpeers" we accepted from this node, to rate-limit the exchange. int64 nLastPexRecv; @@ -723,7 +705,6 @@ public: strBtfAddr.clear(); strBtfMeeting.clear(); nStartingHeight = -1; - vfSubscribe.assign(256, false); // Push a version message /// when NTP implemented, change to just nTime = GetAdjustedTime() @@ -1002,49 +983,6 @@ public: } - void PushRequest(const char* pszCommand, - void (*fn)(void*, CDataStream&), void* param1) - { - uint256 hashReply; - RAND_bytes((unsigned char*)&hashReply, sizeof(hashReply)); - - CRITICAL_BLOCK(cs_mapRequests) - mapRequests[hashReply] = CRequestTracker(fn, param1); - - PushMessage(pszCommand, hashReply); - } - - template - void PushRequest(const char* pszCommand, const T1& a1, - void (*fn)(void*, CDataStream&), void* param1) - { - uint256 hashReply; - RAND_bytes((unsigned char*)&hashReply, sizeof(hashReply)); - - CRITICAL_BLOCK(cs_mapRequests) - mapRequests[hashReply] = CRequestTracker(fn, param1); - - PushMessage(pszCommand, hashReply, a1); - } - - template - void PushRequest(const char* pszCommand, const T1& a1, const T2& a2, - void (*fn)(void*, CDataStream&), void* param1) - { - uint256 hashReply; - RAND_bytes((unsigned char*)&hashReply, sizeof(hashReply)); - - CRITICAL_BLOCK(cs_mapRequests) - mapRequests[hashReply] = CRequestTracker(fn, param1); - - PushMessage(pszCommand, hashReply, a1, a2); - } - - - - bool IsSubscribed(unsigned int nChannel); - void Subscribe(unsigned int nChannel, unsigned int nHops=0); - void CancelSubscribe(unsigned int nChannel); void Disconnect(); }; @@ -1101,50 +1039,8 @@ inline void RelayMessage<>(const CInv& inv, const CDataStream& ss) -// -// Templates for the publish and subscription system. -// The object being published as T& obj needs to have: -// a set setSources member -// specializations of AdvertInsert and AdvertErase -// Currently implemented for CTable and CProduct. -// - -template -void AdvertStartPublish(CNode* pfrom, unsigned int nChannel, unsigned int nHops, T& obj) -{ - // Add to sources - obj.setSources.insert(pfrom->addr.ip); - - if (!AdvertInsert(obj)) - return; - - // Relay - CRITICAL_BLOCK(cs_vNodes) - foreach(CNode* pnode, vNodes) - if (pnode != pfrom && (nHops < PUBLISH_HOPS || pnode->IsSubscribed(nChannel))) - pnode->PushMessage("publish", nChannel, nHops, obj); -} - -template -void AdvertStopPublish(CNode* pfrom, unsigned int nChannel, unsigned int nHops, T& obj) -{ - uint256 hash = obj.GetHash(); - - CRITICAL_BLOCK(cs_vNodes) - foreach(CNode* pnode, vNodes) - if (pnode != pfrom && (nHops < PUBLISH_HOPS || pnode->IsSubscribed(nChannel))) - pnode->PushMessage("pub-cancel", nChannel, nHops, hash); - - AdvertErase(obj); -} - -template -void AdvertRemoveSource(CNode* pfrom, unsigned int nChannel, unsigned int nHops, T& obj) -{ - // Remove a source - obj.setSources.erase(pfrom->addr.ip); - - // If no longer supported by any sources, cancel it - if (obj.setSources.empty()) - AdvertStopPublish(pfrom, nChannel, nHops, obj); -} +// The publish/subscribe advert templates that used to close this file +// (AdvertStartPublish / AdvertStopPublish / AdvertRemoveSource) are gone along +// with the marketplace objects they carried. Nothing ever handled an inbound +// "publish" or "pub-cancel", so the machinery only ever pushed; what it pushed +// was CProduct, which no longer exists. -- cgit v1.2.3