diff --git a/src/llmq/net_quorum.cpp b/src/llmq/net_quorum.cpp index 1727a8f90b0b..fd4879b6ab52 100644 --- a/src/llmq/net_quorum.cpp +++ b/src/llmq/net_quorum.cpp @@ -77,6 +77,14 @@ void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataS } CQuorumDataRequest request; + // An honest QGETDATA is exactly the fixed-size request encoding; nError is + // response-only (QDATA). A smuggled value used to select the *_MISSING branches + // that skip the rate-limit ban, so reject any longer payload by length — this + // also catches an explicitly serialized UNDEFINED byte and trailing garbage. + if (vRecv.size() > GetSerializeSize(request, vRecv.GetVersion())) { + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100, "oversized qgetdata"); + return; + } vRecv >> request; auto sendQDATA = [&](CQuorumDataRequest::Errors nError, diff --git a/test/functional/p2p_quorum_data.py b/test/functional/p2p_quorum_data.py index baa65f98267a..592d933b52c3 100755 --- a/test/functional/p2p_quorum_data.py +++ b/test/functional/p2p_quorum_data.py @@ -376,6 +376,19 @@ def send_bad_qdata_expect_disconnect(bad_qdata): self.restart_mn(mn1) self.wait_for_quorum_data([mn1], 100, quorum_hash, recover=False) + # Requester-supplied nError on QGETDATA is rejected (+100) and disconnects. + # Independent of cleanup / rate-limit state — one poisoned message is enough. + def test_qgetdata_rejects_requester_error(): + for error in (ENCRYPTED_CONTRIBUTIONS_MISSING, 0, 0xFF): + self.log.info(f"Test QGETDATA with requester-supplied nError {error:#x} is disconnected") + p2p_mn = p2p_connection(mn2.get_node(self)) + id_p2p_mn = get_p2p_id(mn2.get_node(self)) + mnauth(mn2.get_node(self), id_p2p_mn, fake_mnauth_2[0], fake_mnauth_2[1]) + wait_for_banscore(mn2.get_node(self), id_p2p_mn, 0) + p2p_mn.send_message(msg_qgetdata(quorum_hash_int, 100, 0x01, error=error)) + self.wait_until(lambda: not p2p_mn.is_connected, timeout=10) + mn2.get_node(self).disconnect_p2ps() + # Test request limiting / banscore increase def test_request_limit(): @@ -531,6 +544,8 @@ def test_qsigshares_inv_oom(): test_watchquorums() test_rpc_quorum_getdata_protx_hash() + # Once: nError rejection does not depend on request-expiry cleanup. + test_qgetdata_rejects_requester_error() test_qsigshares_inv_oom() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index e055e24a205e..1843be6ce083 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -2565,20 +2565,27 @@ def __repr__(self): class msg_qgetdata: - __slots__ = ("quorum_hash", "quorum_type", "data_mask", "protx_hash") + __slots__ = ("quorum_hash", "quorum_type", "data_mask", "protx_hash", "error") msgtype = b"qgetdata" - def __init__(self, quorum_hash=0, quorum_type=-1, data_mask=0, protx_hash=0): + def __init__(self, quorum_hash=0, quorum_type=-1, data_mask=0, protx_hash=0, error=None): self.quorum_hash = quorum_hash self.quorum_type = quorum_type self.data_mask = data_mask self.protx_hash = protx_hash + # error is response-only on the wire. Honest requesters leave it None so + # it is not serialized. Attackers can set it to smuggle a QDATA error + # code into a request (see CQuorumDataRequest SERIALIZE_METHODS). + self.error = error def deserialize(self, f): self.quorum_type = struct.unpack("