fix: bound LLMQ signing vector intake

Author: thepastaclaw

src/llmq/net_signing.cpp

@@ -10,7 +10,6 @@
 #include <llmq/signing_shares.h>
 #include <llmq/signing.h>
 #include <spork.h>
-#include <util/std23.h>
 
 #include <logging.h>
 #include <streams.h>
@@ -24,6 +23,43 @@
 #include <unordered_map>
 
 namespace llmq {
+namespace {
+template <typename T>
+void ReadLimitedVector(CDataStream& stream, std::vector<T>& ret, const size_t max_size)
+{
+    const size_t size{ReadCompactSize(stream)};
+    if (size > max_size) {
+        throw std::ios_base::failure("vector size too large");
+    }
+    ret.clear();
+    ret.reserve(size);
+    while (ret.size() < size) {
+        ret.emplace_back();
+        stream >> ret.back();
+    }
+}
+
+void ReadBatchedSigShares(CDataStream& stream, std::vector<CBatchedSigShares>& ret)
+{
+    const size_t size{ReadCompactSize(stream)};
+    if (size > MAX_MSGS_TOTAL_BATCHED_SIGS) {
+        throw std::ios_base::failure("QBSIGSHARES batch count too large");
+    }
+    ret.clear();
+    ret.reserve(size);
+
+    size_t total_sigs_count{0};
+    while (ret.size() < size) {
+        ret.emplace_back();
+        stream >> ret.back();
+        total_sigs_count += ret.back().sigShares.size();
+        if (total_sigs_count > MAX_MSGS_TOTAL_BATCHED_SIGS) {
+            throw std::ios_base::failure("QBSIGSHARES sig share count too large");
+        }
+    }
+}
+} // namespace
+
 void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv)
 {
     if (msg_type == NetMsgType::QSIGREC) {
@@ -45,13 +81,11 @@ void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CData
 
     if (m_sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) {
         std::vector<CSigShare> receivedSigShares;
-        vRecv >> receivedSigShares;
-
-        if (receivedSigShares.size() > CSigSharesManager::MAX_MSGS_SIG_SHARES) {
-            LogPrint(BCLog::LLMQ_SIGS, "NetSigning::%s -- too many sigs in QSIGSHARE message. cnt=%d, max=%d, node=%d\n",
-                     __func__, receivedSigShares.size(), CSigSharesManager::MAX_MSGS_SIG_SHARES, pfrom.GetId());
+        try {
+            ReadLimitedVector(vRecv, receivedSigShares, CSigSharesManager::MAX_MSGS_SIG_SHARES);
+        } catch (const std::ios_base::failure&) {
             BanNode(pfrom.GetId());
-            return;
+            throw;
         }
 
         for (const auto& sigShare : receivedSigShares) {
@@ -63,13 +97,11 @@ void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CData
 
     if (msg_type == NetMsgType::QSIGSESANN) {
         std::vector<CSigSesAnn> msgs;
-        vRecv >> msgs;
-        if (msgs.size() > CSigSharesManager::MAX_MSGS_CNT_QSIGSESANN) {
-            LogPrint(BCLog::LLMQ_SIGS, /* Continued */
-                     "NetSigning::%s -- too many announcements in QSIGSESANN message. cnt=%d, max=%d, node=%d\n",
-                     __func__, msgs.size(), CSigSharesManager::MAX_MSGS_CNT_QSIGSESANN, pfrom.GetId());
+        try {
+            ReadLimitedVector(vRecv, msgs, CSigSharesManager::MAX_MSGS_CNT_QSIGSESANN);
+        } catch (const std::ios_base::failure&) {
             BanNode(pfrom.GetId());
-            return;
+            throw;
         }
         if (!std::ranges::all_of(msgs, [this, &pfrom](const auto& ann) {
                 return m_shares_manager->ProcessMessageSigSesAnn(pfrom, ann);
@@ -80,17 +112,11 @@ void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CData
     } else if (msg_type == NetMsgType::QSIGSHARESINV || msg_type == NetMsgType::QGETSIGSHARES) {
         std::vector<CSigSharesInv> msgs;
         try {
-            vRecv >> msgs;
+            ReadLimitedVector(vRecv, msgs, CSigSharesManager::MAX_MSGS_CNT_QSIGSHARES);
         } catch (const std::ios_base::failure&) {
             BanNode(pfrom.GetId());
             throw;
         }
-        if (msgs.size() > CSigSharesManager::MAX_MSGS_CNT_QSIGSHARES) {
-            LogPrint(BCLog::LLMQ_SIGS, "NetSigning::%s -- too many invs in %s message. cnt=%d, max=%d, node=%d\n",
-                     __func__, msg_type, msgs.size(), CSigSharesManager::MAX_MSGS_CNT_QSIGSHARES, pfrom.GetId());
-            BanNode(pfrom.GetId());
-            return;
-        }
         if (!std::ranges::all_of(msgs, [this, &pfrom, &msg_type](const auto& inv) {
                 return m_shares_manager->ProcessMessageSigShares(pfrom, inv, msg_type);
             })) {
@@ -99,26 +125,11 @@ void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CData
         }
     } else if (msg_type == NetMsgType::QBSIGSHARES) {
         std::vector<CBatchedSigShares> msgs;
-        const size_t msgs_size{ReadCompactSize(vRecv)};
-        if (msgs_size > MAX_MSGS_TOTAL_BATCHED_SIGS) {
-            LogPrint(BCLog::LLMQ_SIGS, "NetSigning::%s -- too many batches in QBSIGSHARES message. cnt=%d, max=%d, node=%d\n",
-                     __func__, static_cast<int>(msgs_size), MAX_MSGS_TOTAL_BATCHED_SIGS, pfrom.GetId());
-            BanNode(pfrom.GetId());
-            return;
-        }
-        msgs.reserve(msgs_size);
-        while (msgs.size() < msgs_size) {
-            msgs.emplace_back();
-            vRecv >> msgs.back();
-        }
-        const size_t totalSigsCount = std23::ranges::fold_left(msgs, size_t{0}, [](size_t s, const auto& bs) {
-            return s + bs.sigShares.size();
-        });
-        if (totalSigsCount > MAX_MSGS_TOTAL_BATCHED_SIGS) {
-            LogPrint(BCLog::LLMQ_SIGS, "NetSigning::%s -- too many sigs in QBSIGSHARES message. cnt=%d, max=%d, node=%d\n",
-                     __func__, static_cast<int>(totalSigsCount), MAX_MSGS_TOTAL_BATCHED_SIGS, pfrom.GetId());
+        try {
+            ReadBatchedSigShares(vRecv, msgs);
+        } catch (const std::ios_base::failure&) {
             BanNode(pfrom.GetId());
-            return;
+            throw;
         }
         if (!std::ranges::all_of(msgs, [this, &pfrom](const auto& bs) {
                 return m_shares_manager->ProcessMessageBatchedSigShares(pfrom, bs);