fix: bound bitset allocation and stop deserializing DKG messages at intake

src/Makefile.test.include

@@ -137,6 +137,7 @@ BITCOIN_TESTS =\
   test/limitedmap_tests.cpp \
   test/llmq_blockprocessor_tests.cpp \
   test/llmq_dkg_tests.cpp \
+  test/llmq_dkg_intake_tests.cpp \
   test/llmq_chainlock_tests.cpp \
   test/llmq_commitment_tests.cpp \
   test/llmq_hash_tests.cpp \

src/llmq/commitment.cpp

@@ -235,7 +235,7 @@ bool CheckLLMQCommitment(const llmq::UtilParameters& util_params, const CTransac
     }
 
     if (LogAcceptDebug(BCLog::LLMQ)) {
-        // Clamp to validMembers.size() because the wire-format DYNBITSET may be smaller than
+        // Clamp to validMembers.size() because the wire-format bitset may be smaller than
         // llmq_params.size for malformed payloads; VerifySizes() below catches the mismatch.
         std::stringstream ss;
         const auto log_size = std::min<size_t>(llmq_params_opt->size, qcTx.commitment.validMembers.size());
@@ -296,7 +296,7 @@ uint256 BuildCommitmentHash(Consensus::LLMQType llmqType, const uint256& blockHa
     CHashWriter hw(SER_GETHASH, 0);
     hw << llmqType;
     hw << blockHash;
-    hw << DYNBITSET(validMembers);
+    hw << LIMITED_BITSET(validMembers, Consensus::MAX_LLMQ_SIZE);
     hw << pubKey;
     hw << vvecHash;
     return hw.GetHash();

src/llmq/commitment.h

@@ -105,8 +105,8 @@ class CFinalCommitment
             );
         }
         READWRITE(
-                DYNBITSET(obj.signers),
-                DYNBITSET(obj.validMembers),
+                LIMITED_BITSET(obj.signers, Consensus::MAX_LLMQ_SIZE),
+                LIMITED_BITSET(obj.validMembers, Consensus::MAX_LLMQ_SIZE),
                 CBLSPublicKeyVersionWrapper(const_cast<CBLSPublicKey&>(obj.quorumPublicKey), (obj.nVersion == LEGACY_BLS_NON_INDEXED_QUORUM_VERSION || obj.nVersion == LEGACY_BLS_INDEXED_QUORUM_VERSION)),
                 obj.quorumVvecHash,
                 CBLSSignatureVersionWrapper(const_cast<CBLSSignature&>(obj.quorumSig), (obj.nVersion == LEGACY_BLS_NON_INDEXED_QUORUM_VERSION || obj.nVersion == LEGACY_BLS_INDEXED_QUORUM_VERSION)),

src/llmq/dkgmessages.h

@@ -6,6 +6,7 @@
 #define BITCOIN_LLMQ_DKGMESSAGES_H
 
 #include <llmq/commitment.h>
+#include <llmq/params.h>
 
 #include <bls/bls_ies.h>
 #include <hash.h>
@@ -52,7 +53,7 @@ class CDKGContribution
         s >> llmqType;
         s >> quorumHash;
         s >> proTxHash;
-        s >> tmp1;
+        s >> LIMITED_VECTOR(tmp1, Consensus::MAX_LLMQ_SIZE);
         s >> tmp2;
         s >> sig;
 
@@ -90,8 +91,8 @@ class CDKGComplaint
                 obj.llmqType,
                 obj.quorumHash,
                 obj.proTxHash,
-                DYNBITSET(obj.badMembers),
-                DYNBITSET(obj.complainForMembers),
+                LIMITED_BITSET(obj.badMembers, Consensus::MAX_LLMQ_SIZE),
+                LIMITED_BITSET(obj.complainForMembers, Consensus::MAX_LLMQ_SIZE),
                 obj.sig
                 );
     }
@@ -124,7 +125,8 @@ class CDKGJustification
 public:
     SERIALIZE_METHODS(CDKGJustification, obj)
     {
-        READWRITE(obj.llmqType, obj.quorumHash, obj.proTxHash, obj.contributions, obj.sig);
+        READWRITE(obj.llmqType, obj.quorumHash, obj.proTxHash,
+                  LIMITED_VECTOR(obj.contributions, Consensus::MAX_LLMQ_SIZE), obj.sig);
     }
 
     [[nodiscard]] uint256 GetSignHash() const
@@ -170,7 +172,7 @@ class CDKGPrematureCommitment
                 obj.llmqType,
                 obj.quorumHash,
                 obj.proTxHash,
-                DYNBITSET(obj.validMembers),
+                LIMITED_BITSET(obj.validMembers, Consensus::MAX_LLMQ_SIZE),
                 obj.quorumPublicKey,
                 obj.quorumVvecHash,
                 obj.quorumSig,

src/llmq/net_dkg.cpp

@@ -72,48 +72,6 @@ size_t MaxDKGMessageSize(std::string_view msg_type, const Consensus::LLMQParams&
     return cap < HARD_CEILING ? cap : HARD_CEILING;
 }
 
-// Cheap, param-only structural validation of a pushed DKG message, run at intake
-// before retention. Deserializes a COPY of the payload (leaving the caller's bytes
-// intact for the pending queue and its inventory hash) and checks only safe upper
-// bounds derived from quorum params: no member-list lookup and no signature
-// verification, which remain on the DKG worker thread. Deserializing the copy does
-// decompress the BLS points carried in the payload, but that work is bounded by
-// the size cap applied just before this check. Rejects malformed or clearly
-// oversized payloads before retention.
-bool CheckDKGMessageStructure(std::string_view msg_type, const CDataStream& vRecv, const Consensus::LLMQParams& params)
-{
-    const size_t size = params.size > 0 ? static_cast<size_t>(params.size) : 0;
-    const size_t min_size = params.minSize > 0 ? static_cast<size_t>(params.minSize) : 0;
-    const size_t threshold = params.threshold > 0 ? static_cast<size_t>(params.threshold) : 0;
-    try {
-        CDataStream s(vRecv); // copy; deserialization does not advance the caller's stream
-        if (msg_type == NetMsgType::QCONTRIB) {
-            CDKGContribution qc;
-            s >> qc;
-            return qc.vvec != nullptr && qc.vvec->size() == threshold &&
-                   qc.contributions != nullptr &&
-                   qc.contributions->blobs.size() >= min_size &&
-                   qc.contributions->blobs.size() <= size;
-        } else if (msg_type == NetMsgType::QCOMPLAINT) {
-            CDKGComplaint qc;
-            s >> qc;
-            return qc.badMembers.size() == qc.complainForMembers.size() &&
-                   qc.badMembers.size() <= size;
-        } else if (msg_type == NetMsgType::QJUSTIFICATION) {
-            CDKGJustification qj;
-            s >> qj;
-            return qj.contributions.size() <= size;
-        } else if (msg_type == NetMsgType::QPCOMMITMENT) {
-            CDKGPrematureCommitment qc;
-            s >> qc;
-            return qc.validMembers.size() <= size;
-        }
-        return false;
-    } catch (const std::exception&) {
-        return false;
-    }
-}
-
 // returns a set of NodeIds which sent invalid messages
 template <typename Message>
 std::unordered_set<NodeId> BatchVerifyMessageSigs(CDKGSession& session,
@@ -459,13 +417,6 @@ void NetDKG::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStre
         return;
     }
 
-    // Cheap structural pre-validation before retention. Validates a copy so the
-    // original bytes (and their inventory hash) are preserved for the worker.
-    if (!CheckDKGMessageStructure(msg_type, vRecv, llmq_params)) {
-        m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100, "malformed DKG message");
-        return;
-    }
-
     int inv_type = 0;
     if (msg_type == NetMsgType::QCONTRIB)
         inv_type = MSG_QUORUM_CONTRIB;
@@ -492,7 +443,7 @@ void NetDKG::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStre
     //
     // This check runs last so that every pre-existing rejection above -- and the heavier penalty it
     // carries -- is unchanged. It therefore bounds retention and signature verification, not the
-    // parsing and structural validation above, which an unsolicited sender still gets to trigger.
+    // header validation above, which an unsolicited sender still gets to trigger.
     const CInv inv{static_cast<uint32_t>(inv_type), hash};
     if (WITH_LOCK(::cs_main, return m_peer_manager->PeerConsumeGetDataResponse(from, inv)) ==
         GetDataResponse::UNREQUESTED) {

src/serialize.h

@@ -585,16 +585,24 @@ class Wrapper
 template<typename Formatter, typename T>
 static inline Wrapper<Formatter, T&> Using(T&& t) { return Wrapper<Formatter, T&>(t); }
 
-#define DYNBITSET(obj) Using<DynamicBitSetFormatter>(obj)
+#define LIMITED_BITSET(obj,n) Using<LimitedBitSetFormatter<n>>(obj)
 #define AUTOBITSET(obj) Using<AutoBitSetFormatter>(obj)
 #define VARINT_MODE(obj, mode) Using<VarIntFormatter<mode>>(obj)
 #define VARINT(obj) Using<VarIntFormatter<VarIntMode::DEFAULT>>(obj)
 #define COMPACTSIZE(obj) Using<CompactSizeFormatter<true>>(obj)
 #define LIMITED_STRING(obj,n) Using<LimitedStringFormatter<n>>(obj)
 #define LIMITED_VECTOR(obj,n) Using<LimitedVectorFormatter<n>>(obj)
 
-/** TODO: describe DynamicBitSet */
-struct DynamicBitSetFormatter
+/**
+ * Stores a bitset whose length is written on the wire as a CompactSize, followed by the packed bits.
+ *
+ * The declared length is bounded by Limit before the bitset is allocated. Without that bound a
+ * five-byte CompactSize can declare MAX_SIZE bits and make ReadFixedBitSet allocate megabytes for a
+ * payload that never arrives, so callers must pass the largest length the field can legitimately
+ * carry.
+ */
+template<size_t Limit>
+struct LimitedBitSetFormatter
 {
     template<typename Stream>
     void Ser(Stream& s, const std::vector<bool>& vec) const
@@ -606,7 +614,11 @@ struct DynamicBitSetFormatter
     template<typename Stream>
     void Unser(Stream& s, std::vector<bool>& vec)
     {
-        ReadFixedBitSet(s, vec, ReadCompactSize(s));
+        const size_t size = ReadCompactSize(s);
+        if (size > Limit) {
+            throw std::ios_base::failure("Bitset length limit exceeded");
+        }
+        ReadFixedBitSet(s, vec, size);
     }
 };
 

src/test/llmq_commitment_tests.cpp

@@ -102,7 +102,7 @@ BOOST_FIXTURE_TEST_CASE(commitment_check_undersized_bitset_debug_log_test, RegTe
 {
     // Catches the OOB-read regression in CheckLLMQCommitment's debug-log loop
     // by capturing log output rather than relying on undefined behaviour to
-    // trip a sanitizer. The wire-format validMembers DYNBITSET can deserialize
+    // trip a sanitizer. The wire-format validMembers bitset can deserialize
     // smaller than llmq_params.size; before the clamp the loop iterated up to
     // llmq_params.size and emitted v[0], v[1], ... reading past the bitset.
     // With the clamp an empty bitset must produce "validMembers[]".