fix: stop unvalidated governance orphan-vote amplification

src/Makefile.test.include

@@ -121,6 +121,7 @@ BITCOIN_TESTS =\
   test/fs_tests.cpp \
   test/getarg_tests.cpp \
   test/governance_inv_tests.cpp \
+  test/governance_orphan_vote_tests.cpp \
   test/governance_superblock_tests.cpp \
   test/governance_validators_tests.cpp \
   test/governance_vote_wire_tests.cpp \

src/governance/governance.cpp

@@ -19,14 +19,18 @@
 #include <net.h>
 #include <node/interface_ui.h>
 #include <protocol.h>
+#include <random.h>
 #include <timedata.h>
 #include <util/check.h>
 #include <util/time.h>
 #include <validationinterface.h>
 
+#include <algorithm>
 #include <ranges>
 
-const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16";
+// Version 17 drops cmmapOrphanVotes from the on-disk format so an unauthenticated
+// orphan flood cannot survive restart.
+const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-17";
 
 namespace {
 constexpr std::chrono::seconds GOVERNANCE_DELETION_DELAY{10min};
@@ -64,7 +68,7 @@ GovernanceStore::GovernanceStore() :
     mapObjects(),
     mapErasedGovernanceObjects(),
     cmapInvalidVotes(MAX_CACHE_SIZE),
-    cmmapOrphanVotes(MAX_CACHE_SIZE),
+    cmmapOrphanVotes(MAX_ORPHAN_VOTES),
     mapLastMasternodeObject(),
     lastMNListForVotingKeys(std::make_shared<CDeterministicMNList>())
 {
@@ -819,10 +823,61 @@ bool CGovernanceManager::ProcessVote(const CGovernanceVote& vote, CGovernanceExc
         return false;
     }
 
+    const auto tip_mn_list = m_dmnman.GetListAtChainTip();
     auto it = mapObjects.find(nHashGovobj);
     if (it == mapObjects.end()) {
+        // Validate before orphan caching so an unauthenticated peer cannot fill
+        // cmmapOrphanVotes with attacker-chosen parent hashes.
+        // Match the cheap structural checks in CGovernanceVote::IsValid, then
+        // require a tip-list masternode and a verifiable signature.
+        const auto max_time{std::chrono::time_point_cast<std::chrono::seconds>(GetAdjustedTime() + MAX_TIME_FUTURE_DEVIATION)};
+        if (vote.Time() > max_time) {
+            std::string msg{strprintf("CGovernanceManager::%s -- vote is too far ahead of current time, hash = %s",
+                __func__, nHashVote.ToString())};
+            LogPrint(BCLog::GOBJECT, "%s\n", msg);
+            exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_TEMPORARY_ERROR, 20);
+            return false;
+        }
+        if (vote.GetSignal() <= VOTE_SIGNAL_NONE || vote.GetSignal() >= VOTE_SIGNAL_UNKNOWN ||
+            vote.GetOutcome() <= VOTE_OUTCOME_NONE || vote.GetOutcome() >= VOTE_OUTCOME_UNKNOWN) {
+            std::string msg{strprintf("CGovernanceManager::%s -- invalid vote signal/outcome, hash = %s",
+                __func__, nHashVote.ToString())};
+            LogPrint(BCLog::GOBJECT, "%s\n", msg);
+            exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
+            return false;
+        }
+        auto dmn = tip_mn_list.GetMNByCollateral(vote.GetMasternodeOutpoint());
+        if (!dmn) {
+            std::string msg{strprintf("CGovernanceManager::%s -- Unknown Masternode - %s, governance object hash = %s",
+                __func__, vote.GetMasternodeOutpoint().ToStringShort(), nHashGovobj.ToString())};
+            LogPrint(BCLog::GOBJECT, "%s\n", msg);
+            // Match CGovernanceObject::ProcessVote scoring so repeated injection is banned.
+            exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
+            return false;
+        }
+        // FUNDING votes on proposals may use the voting key; everything else uses
+        // the operator BLS key. Parent type is unknown here, so try both.
+        const bool sig_ok = vote.CheckSignature(dmn->pdmnState->keyIDVoting) ||
+                            vote.CheckSignature(dmn->pdmnState->pubKeyOperator.Get());
+        if (!sig_ok) {
+            std::string msg{strprintf("CGovernanceManager::%s -- Invalid vote signature, MN outpoint = %s, "
+                                      "governance object hash = %s, vote hash = %s",
+                __func__, vote.GetMasternodeOutpoint().ToStringShort(), nHashGovobj.ToString(), nHashVote.ToString())};
+            LogPrint(BCLog::GOBJECT, "%s\n", msg);
+            exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_PERMANENT_ERROR, 20);
+            return false;
+        }
+
         std::string msg{strprintf("CGovernanceManager::%s -- Unknown parent object %s, MN outpoint = %s", __func__,
             nHashGovobj.ToString(), vote.GetMasternodeOutpoint().ToStringShort())};
+        // Keep the penalty at zero. Reaching this point means the vote carries a valid
+        // signature from a tip-list masternode, so the only remaining reason we cannot
+        // apply it is that the parent object has not arrived yet - a benign relay race
+        // that happens routinely during governance sync. The peer that forwarded it is
+        // typically an honest relay, and misbehavior scores never decay, so scoring here
+        // would discourage honest peers after DISCOURAGEMENT_THRESHOLD/penalty races.
+        // The flood is bounded by the validation gate above plus MAX_ORPHAN_VOTES, not
+        // by ban scoring.
         exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING);
         if (cmmapOrphanVotes.Insert(nHashGovobj, governance::OrphanVote{vote, Now<NodeSeconds>() + GOVERNANCE_ORPHAN_EXPIRATION_TIME})) {
             hashToRequest = nHashGovobj; // Caller should request this object
@@ -839,7 +894,7 @@ bool CGovernanceManager::ProcessVote(const CGovernanceVote& vote, CGovernanceExc
         return false;
     }
 
-    bool fOk = govobj.ProcessVote(m_mn_metaman, fRateChecksEnabled, m_dmnman.GetListAtChainTip(), vote, exception);
+    bool fOk = govobj.ProcessVote(m_mn_metaman, fRateChecksEnabled, tip_mn_list, vote, exception);
     if (fOk) {
         fOk = cmapVoteToObject.Insert(nHashVote, it->second);
     } else if (exception.GetType() == GOVERNANCE_EXCEPTION_PERMANENT_ERROR && exception.GetNodePenalty() == 20) {
@@ -1103,7 +1158,9 @@ std::vector<uint256> CGovernanceManager::GetOrphanVoteObjectHashes()
         }
     }
 
-    // Get hashes of objects we don't have yet
+    // Get hashes of objects we don't have yet, capped so the scheduler fan-out
+    // stays O(MAX_ORPHAN_OBJECT_REQUESTS_PER_TICK × peers) rather than
+    // O(orphans × peers).
     std::vector<uint256> vecHashesFiltered;
     std::vector<uint256> vecHashes;
     cmmapOrphanVotes.GetKeys(vecHashes);
@@ -1113,6 +1170,16 @@ std::vector<uint256> CGovernanceManager::GetOrphanVoteObjectHashes()
         }
     }
 
+    // Sample randomly rather than truncating: GetKeys() returns ascending uint256
+    // order, so a fixed prefix would request the same numerically-lowest hashes
+    // every tick and starve every other orphan until it expires — a parent whose
+    // hash sorts high would never be requested at all. Matches the shuffle used
+    // for the governance object-vote sync in CSyncManager::RequestGovernanceData().
+    if (vecHashesFiltered.size() > MAX_ORPHAN_OBJECT_REQUESTS_PER_TICK) {
+        Shuffle(vecHashesFiltered.begin(), vecHashesFiltered.end(), FastRandomContext());
+        vecHashesFiltered.resize(MAX_ORPHAN_OBJECT_REQUESTS_PER_TICK);
+    }
+
     return vecHashesFiltered;
 }
 

src/governance/governance.h

@@ -177,6 +177,21 @@ class GovernanceStore
     using txout_m_t = std::map<COutPoint, last_object_rec>;
     using vote_cmm_t = CacheMultiMap<uint256, governance::OrphanVote>;
 
+public:
+    // Bound for orphan-vote amplification. Far below MAX_CACHE_SIZE
+    // so a peer cannot stockpile ~1e6 parent hashes for the
+    // 5-minute MNGOVERNANCESYNC fan-out. Validated-but-orphaned votes still need
+    // a modest window for out-of-order object arrival.
+    static constexpr int MAX_ORPHAN_VOTES = 1000;
+    // Cap scheduler orphan-object requests per 5-minute tick so fan-out stays
+    // O(cap × peers), not O(orphans × peers).
+    static constexpr size_t MAX_ORPHAN_OBJECT_REQUESTS_PER_TICK = 100;
+
+    /** The on-disk format version of governance.dat. Exposed so tests can assert the
+     *  string was bumped alongside a layout change (a stale version would make new
+     *  code misparse an old file instead of discarding it). */
+    static const std::string& GetSerializationVersionString() { return SERIALIZATION_VERSION_STRING; }
+
 protected:
     static constexpr int MAX_CACHE_SIZE = 1000000;
     static const std::string SERIALIZATION_VERSION_STRING;
@@ -205,10 +220,12 @@ class GovernanceStore
     void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store)
     {
         LOCK(cs_store);
+        // Intentionally omit cmmapOrphanVotes: orphan votes are transient recovery
+        // state. Persisting them lets an unauthenticated flood survive restart
+        // and re-drive the scheduler fan-out on every boot.
         s   << SERIALIZATION_VERSION_STRING
             << mapErasedGovernanceObjects
             << cmapInvalidVotes
-            << cmmapOrphanVotes
             << mapObjects
             << mapLastMasternodeObject
             << *lastMNListForVotingKeys;
@@ -228,10 +245,11 @@ class GovernanceStore
 
         s   >> mapErasedGovernanceObjects
             >> cmapInvalidVotes
-            >> cmmapOrphanVotes
             >> mapObjects
             >> mapLastMasternodeObject
             >> *lastMNListForVotingKeys;
+        // Fresh orphan map on load; see Serialize note above.
+        cmmapOrphanVotes.Clear();
     }
 
     void Clear()
@@ -363,8 +381,15 @@ class CGovernanceManager : public GovernanceStore
     // Used by NetGovernance
     std::vector<CInv> FetchRelayInventory() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
     void CheckAndRemove() EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
-    /** Get hashes of governance objects for which we have orphan votes. Also cleans up expired orphans. */
+    /** Get hashes of governance objects for which we have orphan votes. Also cleans up expired orphans.
+     *  Randomly sampled down to MAX_ORPHAN_OBJECT_REQUESTS_PER_TICK for scheduler fan-out safety. */
     [[nodiscard]] std::vector<uint256> GetOrphanVoteObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs_store);
+    /** The bound actually enforced by the orphan-vote cache. Exposed so tests can assert
+     *  the memory-exhaustion bound is in force, not just that the constant exists. */
+    [[nodiscard]] size_t GetOrphanVoteCacheMaxSize() const EXCLUSIVE_LOCKS_REQUIRED(!cs_store)
+    {
+        return WITH_LOCK(cs_store, return cmmapOrphanVotes.GetMaxSize());
+    }
     std::pair<std::vector<uint256>, std::vector<uint256>> FetchGovernanceObjectVotes(
         size_t peers_per_hash_max, int64_t now, std::map<uint256, std::map<CService, int64_t>>& map_asked_recently) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs_store);

src/governance/net_governance.cpp

@@ -45,7 +45,9 @@ void NetGovernance::Schedule(CScheduler& scheduler)
         [this]() -> void {
             if (!m_node_sync.IsSynced()) return;
 
-            // Request governance objects for orphan votes
+            // Request governance objects for orphan votes. GetOrphanVoteObjectHashes() is
+            // already capped; also skip peers whose send buffer is full so a burst of
+            // orphans cannot balloon vSendMsg.
             auto vecOrphanHashes = m_gov_manager.GetOrphanVoteObjectHashes();
             if (!vecOrphanHashes.empty()) {
                 LogPrint(BCLog::GOBJECT, "NetGovernance::Schedule -- requesting %d orphan objects\n",
@@ -54,6 +56,7 @@ void NetGovernance::Schedule(CScheduler& scheduler)
                 for (const uint256& nHash : vecOrphanHashes) {
                     for (CNode* pnode : snap.Nodes()) {
                         if (!pnode->CanRelay()) continue;
+                        if (pnode->fPauseSend) continue;
                         CNetMsgMaker msgMaker(pnode->GetCommonVersion());
                         CBloomFilter filter; // Empty filter - we want the object, not votes
                         m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter));

src/test/governance_inv_tests.cpp

@@ -406,20 +406,24 @@ BOOST_AUTO_TEST_CASE(governance_votes_require_peer_announcement_or_request)
     ProcessInv(*m_node.peerman, *announcing_peer, vote_inv);
     ProcessInv(*m_node.peerman, *second_announcing_peer, vote_inv);
 
+    // Votes with an unknown masternode outpoint are rejected before orphan caching.
+    // No MNGOVERNANCESYNC courtesy request is issued, and the peer is scored once
+    // fully synced.
+    m_node.mn_sync->SwitchToNextAsset();
+    BOOST_REQUIRE(m_node.mn_sync->IsSynced());
+
     connman.FlushSendBuffer(*announcing_peer);
     ProcessGovernanceVote(net_gov, *announcing_peer, vote);
-    BOOST_CHECK_EQUAL(CountQueuedMessages(*announcing_peer, NetMsgType::MNGOVERNANCESYNC), 1U);
-    AssertMisbehaviorScore(*m_node.peerman, *announcing_peer, 0);
+    BOOST_CHECK_EQUAL(CountQueuedMessages(*announcing_peer, NetMsgType::MNGOVERNANCESYNC), 0U);
+    AssertMisbehaviorScore(*m_node.peerman, *announcing_peer, 20);
 
     connman.FlushSendBuffer(*second_announcing_peer);
     ProcessGovernanceVote(net_gov, *second_announcing_peer, vote);
-    // Second announcer: the gate accepts (it independently announced the vote) and consumes
-    // its per-peer request entry. A rejected vote would return before the gate consumes and
-    // leave the entry intact, so this proves the accept path independently of the (deduped)
-    // orphan-request side effect.
+    // Second announcer: the gate still accepts (it independently announced the vote) and
+    // consumes its per-peer request entry even though ProcessVote then rejects the vote.
     BOOST_CHECK(!WITH_LOCK(::cs_main,
                            return m_node.peerman->PeerConsumeObjectRequest(second_announcing_peer->GetId(), vote_inv)));
-    AssertMisbehaviorScore(*m_node.peerman, *second_announcing_peer, 0);
+    AssertMisbehaviorScore(*m_node.peerman, *second_announcing_peer, 20);
 
     m_node.peerman->FinalizeNode(*announcing_peer);
     m_node.peerman->FinalizeNode(*second_announcing_peer);
@@ -458,14 +462,20 @@ BOOST_AUTO_TEST_CASE(governance_vote_authorization_survives_unsynced_drop)
     ProcessGovernanceVote(net_gov, *peer, vote);
     BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::MNGOVERNANCESYNC), 0U);
 
-    // Back in sync, the retransmit is still authorized: ProcessVote runs and (orphan parent)
-    // requests the missing object. Had the unsynced drop consumed the request, the gate would now
-    // reject the vote as unrequested and send no MNGOVERNANCESYNC.
+    // Back in sync, the retransmit is still authorized: the gate consumes the request and
+    // ProcessVote runs. The vote uses an unknown MN outpoint so it is rejected with a
+    // misbehavior score rather than cached as an orphan. Had the unsynced
+    // drop consumed the request, the gate would now reject the vote as unrequested and leave
+    // the misbehavior score at 0.
     m_node.mn_sync->SwitchToNextAsset();
     BOOST_REQUIRE(m_node.mn_sync->IsBlockchainSynced());
+    // Advance fully to FINISHED so the penalty path applies (gated on IsSynced()).
+    m_node.mn_sync->SwitchToNextAsset();
+    BOOST_REQUIRE(m_node.mn_sync->IsSynced());
     connman.FlushSendBuffer(*peer);
     ProcessGovernanceVote(net_gov, *peer, vote);
-    BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::MNGOVERNANCESYNC), 1U);
+    BOOST_CHECK_EQUAL(CountQueuedMessages(*peer, NetMsgType::MNGOVERNANCESYNC), 0U);
+    AssertMisbehaviorScore(*m_node.peerman, *peer, 20);
 
     m_node.peerman->FinalizeNode(*peer);
     chainstate.ResetIbd();