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/rpc_getmerkleblocks.py b/test/functional/rpc_getmerkleblocks.py new file mode 100755 index 000000000000..36948df28608 --- /dev/null +++ b/test/functional/rpc_getmerkleblocks.py @@ -0,0 +1,90 @@ +#!/usr/bin/env python3 +# 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. + +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_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 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")