1 files changed, 37 insertions, 78 deletions

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<unsigned int, vector<unsigned char> > 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<string,string> 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<uint256, CRequestTracker>::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")