fix: penalize oversized notfound messages

src/net_processing.cpp

@@ -5464,24 +5464,27 @@ void PeerManagerImpl::ProcessMessage(
     }
     if (msg_type == NetMsgType::NOTFOUND) {
         // Remove the NOTFOUND transactions from the peer
-        LOCK(cs_main);
-        CNodeState *state = State(pfrom.GetId());
         std::vector<CInv> vInv;
         vRecv >> vInv;
-        if (vInv.size() <= MAX_PEER_OBJECT_IN_FLIGHT + MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
-            for (CInv &inv : vInv) {
-                if (inv.IsKnownType()) {
-                    // If we receive a NOTFOUND message for a txid we requested, erase
-                    // it from our data structures for this peer.
-                    auto in_flight_it = state->m_object_download.m_object_in_flight.find(inv);
-                    if (in_flight_it == state->m_object_download.m_object_in_flight.end()) {
-                        // Skip any further work if this is a spurious NOTFOUND
-                        // message.
-                        continue;
-                    }
-                    state->m_object_download.m_object_in_flight.erase(in_flight_it);
-                    state->m_object_download.m_object_announced.erase(inv);
+        if (vInv.size() > MAX_PEER_OBJECT_IN_FLIGHT + MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
+            Misbehaving(pfrom.GetId(), 20, strprintf("notfound message size = %u", vInv.size()));
+            return;
+        }
+
+        LOCK(cs_main);
+        CNodeState *state = State(pfrom.GetId());
+        for (CInv &inv : vInv) {
+            if (inv.IsKnownType()) {
+                // If we receive a NOTFOUND message for a txid we requested, erase
+                // it from our data structures for this peer.
+                auto in_flight_it = state->m_object_download.m_object_in_flight.find(inv);
+                if (in_flight_it == state->m_object_download.m_object_in_flight.end()) {
+                    // Skip any further work if this is a spurious NOTFOUND
+                    // message.
+                    continue;
                 }
+                state->m_object_download.m_object_in_flight.erase(in_flight_it);
+                state->m_object_download.m_object_announced.erase(inv);
             }
         }
         return;

test/functional/p2p_tx_download.py

@@ -40,6 +40,8 @@ def on_getdata(self, message):
 MAX_GETDATA_RANDOM_DELAY = 2  # seconds
 INBOUND_PEER_TX_DELAY = 2  # seconds
 MAX_GETDATA_IN_FLIGHT = 100
+MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16
+MAX_NOTFOUND_SIZE = MAX_GETDATA_IN_FLIGHT + MAX_BLOCKS_IN_TRANSIT_PER_PEER
 TX_EXPIRY_INTERVAL = GETDATA_TX_INTERVAL * 10
 
 # Python test constants
@@ -142,13 +144,21 @@ def test_spurious_notfound(self):
         self.log.info('Check that spurious notfound is ignored')
         self.nodes[0].p2ps[0].send_message(msg_notfound(vec=[CInv(1, 1)]))
 
+    def test_oversized_notfound(self):
+        self.log.info('Check that oversized notfound increases misbehavior score')
+        oversized_notfound_count = MAX_NOTFOUND_SIZE + 1
+        invs = [CInv(t=1, h=i) for i in range(oversized_notfound_count)]
+        with self.nodes[0].assert_debug_log(["Misbehaving", f"notfound message size = {oversized_notfound_count}"]):
+            self.nodes[0].p2ps[0].send_message(msg_notfound(vec=invs))
+            self.nodes[0].p2ps[0].sync_with_ping()
+
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
         self.wallet.rescan_utxos()
 
         # Run each test against new bitcoind instances, as setting mocktimes has long-term effects on when
         # the next trickle relay event happens.
-        for test in [self.test_spurious_notfound, self.test_in_flight_max, self.test_inv_block, self.test_tx_requests]:
+        for test in [self.test_spurious_notfound, self.test_oversized_notfound, self.test_in_flight_max, self.test_inv_block, self.test_tx_requests]:
             self.stop_nodes()
             self.start_nodes()
             self.connect_nodes(1, 0)