From 28d2c6c7ca0f02909a2d21d67632608fedb412f7 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Fri, 10 Jul 2026 14:15:51 -0500 Subject: [PATCH 1/3] fix(net): bound CBloomFilter and filteradd vectors before allocation A peer-controlled CBloomFilter and the FILTERADD data element were decoded in full before their size was checked. The length is a CompactSize up to MAX_SIZE, so a raw count with the bytes omitted forced a large speculative allocation and then threw std::ios_base::failure into net_processing's outer message-processing catch, which drops without punishing the peer and so allowed indefinite replay. Bound the wire count before any element is decoded using LIMITED_VECTOR: CBloomFilter::vData at MAX_BLOOM_FILTER_SIZE in its serialization method and the FILTERADD data element at MAX_SCRIPT_ELEMENT_SIZE at the call site. The wire format is unchanged, so IsWithinSizeConstraints() still guards the exact boundary and nHashFuncs. At FILTERLOAD, FILTERADD, and the MNGOVERNANCESYNC consumer of the shared type, catch the local std::ios_base::failure and apply the existing 100-point punishment; the govsync catch is scoped to the filter read so unrelated nProp handling is unchanged. Because CBloomFilter is now globally bounded, getmerkleblocks prechecks the leading vData count without consuming the stream: a fully present oversized filter keeps its historical RPC_INVALID_PARAMETER response, a truncated oversized declaration reproduces the original DataStream end-of-data error, and malformed, noncanonical, and above-MAX_SIZE prefixes fall through to normal deserialization unchanged. Add exact-trigger regressions to p2p_filter.py and p2p_govsync_bloom.py (via msg_generic) that declare a CompactSize(MAX_SIZE) length with the bytes omitted and assert the peer is punished; existing valid-boundary coverage is retained. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/common/bloom.h | 4 +++- src/governance/net_governance.cpp | 11 +++++++-- src/net_processing.cpp | 24 +++++++++++++------ src/rpc/blockchain.cpp | 28 ++++++++++++++++++++++ test/functional/p2p_filter.py | 15 ++++++++++++ test/functional/p2p_govsync_bloom.py | 14 ++++++++++- test/functional/test_framework/messages.py | 2 +- 7 files changed, 86 insertions(+), 12 deletions(-) diff --git a/src/common/bloom.h b/src/common/bloom.h index bc51d3445dbc..70a0d5c42e3d 100644 --- a/src/common/bloom.h +++ b/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 vKey); void insert(const COutPoint& outpoint); diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index da27cf57b453..ea3574b9ea51 100644 --- a/src/governance/net_governance.cpp +++ b/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 diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f61add3b6d95..13707221d4ef 100644 --- a/src/net_processing.cpp +++ b/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 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); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 700daad3a328..ca2a0f039c0a 100644 --- a/src/rpc/blockchain.cpp +++ b/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"); diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 7ed23541638b..594148eca424 100755 --- a/test/functional/p2p_filter.py +++ b/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): diff --git a/test/functional/p2p_govsync_bloom.py b/test/functional/p2p_govsync_bloom.py index 9ade5f5ba64a..257bf44ee462 100755 --- a/test/functional/p2p_govsync_bloom.py +++ b/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() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 87b260758398..5f66dc6af064 100755 --- a/test/functional/test_framework/messages.py +++ b/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 From 862e7b0e89092e68c463245cb724a31947b460ae Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Fri, 10 Jul 2026 20:14:27 -0500 Subject: [PATCH 2/3] test: cover getmerkleblocks oversized bloom-filter prechecks getmerkleblocks hand-parses the leading CompactSize vData count so that, now that CBloomFilter deserialization is bounded, an oversized filter still reproduces the historical RPC errors instead of a generic length-limit failure. That byte-level precheck (distinguishing a fully present oversized filter from a truncated one, including the trailing fixed-field boundary) had no automated coverage. Add rpc_getmerkleblocks.py, which asserts: - a fully present oversized filter -> RPC_INVALID_PARAMETER ("Filter is not within size constraints"); - an oversized declaration with the vData bytes omitted, or with the trailer short by one byte, -> the original DataStream end-of-data RPC_MISC_ERROR; - a well-formed in-bounds filter falls through and is handled normally. Register the test in BASE_SCRIPTS and drop the now-satisfied getmerkleblocks RPC-coverage TODO, since the passing filter call exercises the command. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/functional/rpc_getmerkleblocks.py | 90 ++++++++++++++++++++++++++ test/functional/test_runner.py | 3 +- 2 files changed, 91 insertions(+), 2 deletions(-) create mode 100755 test/functional/rpc_getmerkleblocks.py diff --git a/test/functional/rpc_getmerkleblocks.py b/test/functional/rpc_getmerkleblocks.py new file mode 100755 index 000000000000..52e351d4d227 --- /dev/null +++ b/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() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e303013a6b75..9c785bef4e27 100755 --- a/test/functional/test_runner.py +++ b/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") From 5b6ac384d7f3682f9e0d96af397c1ad468cb118b Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Sat, 11 Jul 2026 14:42:04 -0500 Subject: [PATCH 3/3] test: fix getmerkleblocks copyright year --- test/functional/rpc_getmerkleblocks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/rpc_getmerkleblocks.py b/test/functional/rpc_getmerkleblocks.py index 52e351d4d227..36948df28608 100755 --- a/test/functional/rpc_getmerkleblocks.py +++ b/test/functional/rpc_getmerkleblocks.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2024 The Dash Core developers +# Copyright (c) 2026 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.