fix(net): bound bloom message vectors before allocation

src/common/bloom.h

@@ -73,7 +73,9 @@ class CBloomFilter
     CBloomFilter(const unsigned int nElements, const double nFPRate, const unsigned int nTweak, unsigned char nFlagsIn);
     CBloomFilter() : nHashFuncs(0), nTweak(0), nFlags(0) {}
 
-    SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(obj.vData, obj.nHashFuncs, obj.nTweak, obj.nFlags); }
+    // Bound vData at MAX_BLOOM_FILTER_SIZE before allocation. Wire format is unchanged;
+    // IsWithinSizeConstraints() still guards the exact boundary and nHashFuncs.
+    SERIALIZE_METHODS(CBloomFilter, obj) { READWRITE(LIMITED_VECTOR(obj.vData, MAX_BLOOM_FILTER_SIZE), obj.nHashFuncs, obj.nTweak, obj.nFlags); }
 
     void insert(Span<const unsigned char> vKey);
     void insert(const COutPoint& outpoint);

src/governance/net_governance.cpp

@@ -90,9 +90,16 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa
         if (!m_node_sync.IsSynced()) return;
 
         uint256 nProp;
-        CBloomFilter filter;
         vRecv >> nProp;
-        vRecv >> filter;
+
+        CBloomFilter filter;
+        try {
+            vRecv >> filter;
+        } catch (const std::ios_base::failure& e) {
+            // An oversized filter now throws pre-allocation; punish here instead of the outer catch.
+            m_peer_manager->PeerMisbehaving(peer.GetId(), 100, strprintf("misformatted govsync bloom filter. peer=%d error=%s", peer.GetId(), e.what()));
+            return;
+        }
 
         // The per-object vote-sync path tests this peer-supplied filter against every
         // cached vote (CBloomFilter::contains() loops nHashFuncs times). An unbounded

src/net_processing.cpp

@@ -5398,7 +5398,13 @@ void PeerManagerImpl::ProcessMessage(
             return;
         }
         CBloomFilter filter;
-        vRecv >> filter;
+        try {
+            vRecv >> filter;
+        } catch (const std::ios_base::failure& e) {
+            // An oversized filter now throws pre-allocation; punish here instead of the outer catch.
+            Misbehaving(*peer, 100, strprintf("misformatted bloom filter. peer=%d error=%s", pfrom.GetId(), e.what()));
+            return;
+        }
 
         if (!filter.IsWithinSizeConstraints())
         {
@@ -5422,15 +5428,19 @@ void PeerManagerImpl::ProcessMessage(
             pfrom.fDisconnect = true;
             return;
         }
+        // Nodes must NEVER send a data item > 520 bytes (the max size for a script data object,
+        // and thus, the maximum size any matched object can have) in a filteradd message. Bound
+        // the declared length before allocation and punish a bad count here, not the outer catch.
         std::vector<unsigned char> vData;
-        vRecv >> vData;
+        try {
+            vRecv >> LIMITED_VECTOR(vData, MAX_SCRIPT_ELEMENT_SIZE);
+        } catch (const std::ios_base::failure& e) {
+            Misbehaving(*peer, 100, strprintf("bad filteradd message. peer=%d error=%s", pfrom.GetId(), e.what()));
+            return;
+        }
 
-        // Nodes must NEVER send a data item > 520 bytes (the max size for a script data object,
-        // and thus, the maximum size any matched object can have) in a filteradd message
         bool bad = false;
-        if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) {
-            bad = true;
-        } else if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) {
+        if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) {
             LOCK(tx_relay->m_bloom_filter_mutex);
             if (tx_relay->m_bloom_filter) {
                 tx_relay->m_bloom_filter->insert(vData);

src/rpc/blockchain.cpp

@@ -852,6 +852,34 @@ static RPCHelpMan getmerkleblocks()
     CBloomFilter filter;
     std::string strFilter = request.params[0].get_str();
     CDataStream ssBloomFilter(ParseHex(strFilter), SER_NETWORK, PROTOCOL_VERSION);
+    // CBloomFilter deserialization is now bounded, so an oversized vData count throws a generic
+    // length-limit error instead of the historical response. Preserve the old behavior by
+    // prechecking the leading vData count without consuming the stream: a fully present oversized
+    // filter (all vData bytes plus the fixed trailing fields) historically deserialized and then
+    // failed IsWithinSizeConstraints(), while a truncated one raised DataStream end-of-data. Only
+    // these two well-formed-count cases are handled here; malformed, noncanonical, and
+    // above-MAX_SIZE prefixes fall through to normal deserialization, which reproduces them.
+    enum { PRECHECK_OK, OVERSIZED_COMPLETE, OVERSIZED_TRUNCATED } precheck{PRECHECK_OK};
+    try {
+        SpanReader prefix{SER_NETWORK, PROTOCOL_VERSION, MakeUCharSpan(ssBloomFilter)};
+        const uint64_t vdata_size{ReadCompactSize(prefix)};
+        if (vdata_size > MAX_BLOOM_FILTER_SIZE) {
+            // Wire size of the fixed fields after vData: nHashFuncs + nTweak (uint32) + nFlags (uint8).
+            constexpr uint64_t FILTER_TRAILER_SIZE{2 * sizeof(uint32_t) + sizeof(uint8_t)};
+            const uint64_t remaining{prefix.size()};
+            const bool complete{remaining >= vdata_size && remaining - vdata_size >= FILTER_TRAILER_SIZE};
+            precheck = complete ? OVERSIZED_COMPLETE : OVERSIZED_TRUNCATED;
+        }
+    } catch (const std::ios_base::failure&) {
+        // Malformed/noncanonical/above-MAX_SIZE count: leave PRECHECK_OK; deserialization reproduces it.
+    }
+    if (precheck == OVERSIZED_COMPLETE) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Filter is not within size constraints");
+    }
+    if (precheck == OVERSIZED_TRUNCATED) {
+        // Reproduce the original end-of-data failure without allocating the oversized vData.
+        throw std::ios_base::failure("DataStream::read(): end of data");
+    }
     ssBloomFilter >> filter;
     if (!filter.IsWithinSizeConstraints()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Filter is not within size constraints");

test/functional/p2p_filter.py

@@ -16,9 +16,11 @@
     msg_filteradd,
     msg_filterclear,
     msg_filterload,
+    msg_generic,
     msg_getdata,
     msg_mempool,
     msg_version,
+    ser_compact_size,
 )
 from test_framework.p2p import (
     P2PInterface,
@@ -34,6 +36,10 @@
     getnewdestination,
 )
 
+# serialize.h MAX_SIZE: the largest count ReadCompactSize() accepts, so a declared
+# vector length of this value reaches the vData/data cap, not the compact-size guard.
+MAX_SIZE = 0x02000000
+
 
 class P2PBloomFilter(P2PInterface):
     # This is a P2SH watch-only wallet
@@ -112,6 +118,11 @@ def test_size_limits(self, filter_peer):
             filter_peer.send_and_ping(msg_filterload(data=b'\xbb'*(MAX_BLOOM_FILTER_SIZE)))
         filter_peer.send_and_ping(msg_filterclear())
 
+        self.log.info('Check that a filterload declaring an oversized vData length with the bytes omitted is rejected before allocation')
+        # Without the cap this would fall into the outer catch (no Misbehaving) after a large allocation.
+        with self.nodes[0].assert_debug_log(['Misbehaving']):
+            filter_peer.send_and_ping(msg_generic(b'filterload', ser_compact_size(MAX_SIZE)))
+
         self.log.info('Check that filter with too many hash functions is rejected')
         with self.nodes[0].assert_debug_log(['Misbehaving']):
             filter_peer.send_and_ping(msg_filterload(data=b'\xaa', nHashFuncs=MAX_BLOOM_HASH_FUNCS+1))
@@ -129,6 +140,10 @@ def test_size_limits(self, filter_peer):
         with self.nodes[0].assert_debug_log(['Misbehaving']):
             filter_peer.send_and_ping(msg_filteradd(data=b'\xcc'*(MAX_SCRIPT_ELEMENT_SIZE+1)))
 
+        self.log.info('Check that a filteradd declaring an oversized data length with the bytes omitted is rejected before allocation')
+        with self.nodes[0].assert_debug_log(['Misbehaving']):
+            filter_peer.send_and_ping(msg_generic(b'filteradd', ser_compact_size(MAX_SIZE)))
+
         filter_peer.send_and_ping(msg_filterclear())
 
     def test_msg_mempool(self):

test/functional/p2p_govsync_bloom.py

@@ -13,13 +13,16 @@
 """
 import struct
 
-from test_framework.messages import ser_string, ser_uint256
+from test_framework.messages import msg_generic, ser_compact_size, ser_string, ser_uint256
 from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import force_finish_mnsync
 
 # CBloomFilter size constraints (src/common/bloom.h).
 MAX_HASH_FUNCS = 50
+# serialize.h MAX_SIZE: the largest count ReadCompactSize() accepts, so a declared
+# vData length of this value reaches the vData cap, not the compact-size guard.
+MAX_SIZE = 0x02000000
 
 
 class msg_govsync:
@@ -65,6 +68,15 @@ def run_test(self):
         bad_peer.send_message(msg_govsync(n_hash_funcs=0xFFFFFFFF))
         bad_peer.wait_for_disconnect()
 
+        self.log.info("A govsync request declaring an oversized filter vData length with the bytes omitted is rejected before allocation")
+        # nProp (32 bytes) then a CompactSize(MAX_SIZE) vData length with no bytes. Without the
+        # cap this would fall into net_processing's outer catch (no Misbehaving, no disconnect).
+        raw_peer = node.add_p2p_connection(P2PInterface())
+        raw_payload = ser_uint256(0) + ser_compact_size(MAX_SIZE)
+        with node.assert_debug_log(['Misbehaving']):
+            raw_peer.send_message(msg_generic(b'govsync', raw_payload))
+            raw_peer.wait_for_disconnect()
+
 
 if __name__ == '__main__':
     GovsyncBloomCapTest().main()

test/functional/test_framework/messages.py

@@ -1904,7 +1904,7 @@ def __repr__(self):
 # for cases where a user needs tighter control over what is sent over the wire
 # note that the user must supply the name of the msgtype, and the data
 class msg_generic:
-    __slots__ = ("data")
+    __slots__ = ("msgtype", "data")
 
     def __init__(self, msgtype, data=None):
         self.msgtype = msgtype