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/rpc_getmerkleblocks.py

@@ -0,0 +1,90 @@
+#!/usr/bin/env python3
+# Copyright (c) 2024 The Dash Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test the getmerkleblocks RPC bloom-filter size prechecks.
+
+Now that CBloomFilter deserialization is bounded (LIMITED_VECTOR on vData),
+getmerkleblocks parses the leading CompactSize vData count by hand so that an
+oversized filter still reproduces the historical RPC errors instead of a
+generic length-limit failure (see the precheck in src/rpc/blockchain.cpp).
+
+This pins that byte-level boundary behaviour:
+
+  - a fully present oversized filter (all vData bytes plus the fixed trailer)
+    historically deserialized and then failed IsWithinSizeConstraints(), so it
+    must still raise RPC_INVALID_PARAMETER ("Filter is not within size
+    constraints");
+  - an oversized declaration with the vData bytes omitted, or with the trailing
+    fixed fields short by even one byte, historically raised a DataStream
+    end-of-data failure, so it must still surface that RPC_MISC_ERROR;
+  - a well-formed in-bounds filter is accepted (the precheck falls through) and
+    returns an array.
+"""
+
+from test_framework.messages import ser_compact_size
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal, assert_raises_rpc_error
+
+# Keep in sync with MAX_BLOOM_FILTER_SIZE in src/common/bloom.h.
+MAX_BLOOM_FILTER_SIZE = 36000
+# Wire size of the fixed fields serialized after vData:
+# nHashFuncs (uint32) + nTweak (uint32) + nFlags (uint8).
+FILTER_TRAILER_SIZE = 9
+
+
+def raw_filter(declared_vdata_size, vdata_bytes, trailer_bytes):
+    """Build a raw bloom-filter wire blob with an arbitrary declared vData
+    CompactSize count and an arbitrary number of vData/trailer bytes actually
+    present, so truncation boundaries can be exercised directly."""
+    return (
+        ser_compact_size(declared_vdata_size)
+        + b"\x00" * vdata_bytes
+        + b"\x00" * trailer_bytes
+    ).hex()
+
+
+class GetMerkleBlocksTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 1
+
+    def run_test(self):
+        node = self.nodes[0]
+        genesis_hash = node.getblockhash(0)
+        oversized = MAX_BLOOM_FILTER_SIZE + 1
+
+        # Fully present oversized filter: deserialization would succeed, so the
+        # historical response is the IsWithinSizeConstraints() rejection.
+        complete = raw_filter(oversized, oversized, FILTER_TRAILER_SIZE)
+        assert_raises_rpc_error(-8, "Filter is not within size constraints",
+                                node.getmerkleblocks, complete, genesis_hash, 1)
+
+        # Oversized declaration with the vData bytes omitted: reading vData hits
+        # end-of-data.
+        truncated_body = raw_filter(oversized, 0, 0)
+        assert_raises_rpc_error(-1, "DataStream::read(): end of data",
+                                node.getmerkleblocks, truncated_body, genesis_hash, 1)
+
+        # All vData present but the trailer short by one byte: exercises the
+        # FILTER_TRAILER_SIZE boundary of the precheck, still end-of-data.
+        truncated_trailer = raw_filter(oversized, oversized, FILTER_TRAILER_SIZE - 1)
+        assert_raises_rpc_error(-1, "DataStream::read(): end of data",
+                                node.getmerkleblocks, truncated_trailer, genesis_hash, 1)
+
+        # A well-formed in-bounds filter falls through the precheck and is
+        # handled normally. Build one explicitly (3-byte all-zero vData,
+        # nHashFuncs=1, nTweak=0, nFlags=0); it matches nothing, so the genesis
+        # block yields no merkleblocks.
+        valid = (
+            ser_compact_size(3)
+            + b"\x00\x00\x00"
+            + (1).to_bytes(4, "little")   # nHashFuncs
+            + (0).to_bytes(4, "little")   # nTweak
+            + b"\x00"                      # nFlags
+        ).hex()
+        assert_equal(node.getmerkleblocks(valid, genesis_hash, 1), [])
+
+
+if __name__ == '__main__':
+    GetMerkleBlocksTest().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

test/functional/test_runner.py

@@ -282,6 +282,7 @@
     'wallet_backwards_compatibility.py --descriptors',
     'wallet_txn_clone.py --mineblock',
     'rpc_getblockfilter.py',
+    'rpc_getmerkleblocks.py',
     'rpc_getblockfrompeer.py',
     'rpc_invalidateblock.py',
     'feature_txindex.py',
@@ -936,8 +937,6 @@ def _get_uncovered_rpc_commands(self):
         covered_cmds.add('voteraw')
         # TODO: implement functional tests for importelectrumwallet
         covered_cmds.add('importelectrumwallet')
-        # TODO: implement functional tests for getmerkleblocks
-        covered_cmds.add('getmerkleblocks')
 
         if not os.path.isfile(coverage_ref_filename):
             raise RuntimeError("No coverage reference found")