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/main.cpp | 115 +++++++++++++++++++---------------------------------------- 1 file changed, 37 insertions(+), 78 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 62068d2..efff96c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2782,9 +2782,10 @@ bool AlreadyHave(CTxDB& txdb, const CInv& inv) { case MSG_TX: return mapTransactions.count(inv.hash) || txdb.ContainsTx(inv.hash); case MSG_BLOCK: return mapBlockIndex.count(inv.hash) || mapOrphanBlocks.count(inv.hash); - case MSG_REVIEW: return true; - case MSG_PRODUCT: return mapProducts.count(inv.hash); } + // MSG_REVIEW / MSG_PRODUCT / MSG_TABLE are the 2009 marketplace inventory + // types. Nothing announces or serves them any more (see ProcessMessage), + // so they fall through to the default and are never requested. // Don't know what it is, just say we already got one return true; } @@ -2942,7 +2943,6 @@ bool ProcessMessages(CNode* pfrom) bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) { - static map > mapReuseKey; if (LogAcceptsCategory("net")) { printf("received: %-12s (%d bytes) ", strCommand.c_str(), (int)vRecv.size()); for (int i = 0; i < min(vRecv.size(), (unsigned int)25); i++) printf("%02x ", vRecv[i] & 0xff); printf("\n"); } if (nDropMessagesTest > 0 && GetRand(nDropMessagesTest) == 0) { @@ -3181,22 +3181,18 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) } - else if (strCommand == "review") - { - CDataStream vMsg(vRecv); - CReview review; - vRecv >> review; - - CInv inv(MSG_REVIEW, review.GetHash()); - pfrom->AddInventoryKnown(inv); - - if (review.AcceptReview()) - { - // Relay the original message as-is in case it's a higher version than we know how to parse - RelayMessage(inv, vMsg); - mapAlreadyAskedFor.erase(inv); - } - } + // "review" used to be handled here. It took a CReview off the wire from an + // unauthenticated peer, checked a signature the sender had made with a key + // of its own choosing -- which proves possession of that key and authorises + // nothing -- and then appended the message to reviews.dat by reading the + // whole vector for the target hash, pushing one entry, and writing all of + // it back. That is quadratic disk for a stream an attacker controls, on a + // record whose mapValue is an unbounded map read straight + // from the socket, followed by AddAtomsAndPropagate walking a link graph + // the same peer supplied, and finally RelayMessage forwarding it to every + // other node. Unbounded remote disk growth with network amplification and + // no authorisation anywhere in the path. The marketplace it belonged to has + // not existed since 2009; the handler is gone rather than bounded. else if (strCommand == "block") @@ -3229,65 +3225,28 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) } - else if (strCommand == "checkorder") - { - uint256 hashReply; - CWalletTx order; - vRecv >> hashReply >> order; - - /// we have a chance to check the order here - - // Keep giving the same key to the same ip until they use it - if (!mapReuseKey.count(pfrom->addr.ip)) - mapReuseKey[pfrom->addr.ip] = GenerateNewKey(); - - // Send back approval of order and pubkey to use - CScript scriptPubKey; - scriptPubKey << mapReuseKey[pfrom->addr.ip] << OP_CHECKSIG; - pfrom->PushMessage("reply", hashReply, (int)0, scriptPubKey); - } - - - else if (strCommand == "submitorder") - { - uint256 hashReply; - CWalletTx wtxNew; - vRecv >> hashReply >> wtxNew; - - // Broadcast - if (!wtxNew.AcceptWalletTransaction()) - { - pfrom->PushMessage("reply", hashReply, (int)1); - return error("submitorder AcceptWalletTransaction() failed, returning error 1"); - } - wtxNew.fTimeReceivedIsTxTime = true; - AddToWallet(wtxNew); - wtxNew.RelayWalletTransaction(); - mapReuseKey.erase(pfrom->addr.ip); - - // Send back confirmation - pfrom->PushMessage("reply", hashReply, (int)0); - } - - - else if (strCommand == "reply") - { - uint256 hashReply; - vRecv >> hashReply; - - CRequestTracker tracker; - CRITICAL_BLOCK(pfrom->cs_mapRequests) - { - map::iterator mi = pfrom->mapRequests.find(hashReply); - if (mi != pfrom->mapRequests.end()) - { - tracker = (*mi).second; - pfrom->mapRequests.erase(mi); - } - } - if (!tracker.IsNull()) - tracker.fn(tracker.param1, vRecv); - } + // "checkorder", "submitorder" and "reply" -- the 2009 buy-from-a-merchant + // exchange -- used to be handled here. All three are gone. + // + // "checkorder" called GenerateNewKey() for any peer that asked, which is + // AddKey() and a wallet.dat write, and handed the pubkey back. The comment + // said "keep giving the same key to the same ip until they use it", and on + // a 2009 network of routable IPs that held. It does not hold here: every + // inbound peer arrives through a rendezvous tunnel and is tagged with + // BtfMarkerAddr() over three random bytes (net.cpp), so addr.ip is freshly + // random per connection and the cache never hits. Reconnect in a loop and + // the node mints a key per connection -- unbounded wallet growth, unbounded + // growth of the static map that held them, and every one of those keys + // missing from every backup taken beforehand, which for this pre-BIP32 + // wallet is how coins are lost. + // + // "submitorder" took a CWalletTx from an unauthenticated peer and, if it + // was merely a valid transaction, AddToWallet()'d it. Valid is not the same + // as ours: any peer could write chosen transactions into wallet.dat. + // + // "reply" dispatched to a CRequestTracker callback. Nothing in this tree + // has ever called PushRequest, so mapRequests was permanently empty and the + // handler could only ever be a parser for hostile input. else if (strCommand == "ping") -- cgit v1.2.3