From f2f1dd49a6cb1260a89d406b906d270e0bfe27dd Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 8 Oct 2026 21:43:43 -0500 Subject: [PATCH 1/2] fix: reset the governance cache when upgrading Bump the governance cache version so every node discards its existing governance.dat once and syncs governance objects and votes again from the network. Proposal signatures are never checked, so a cache can hold objects whose signature field is arbitrarily large; dropping the old cache lets the next commit bound that field on every read without failing to load an existing cache. The format change also removes the historical invalid-vote-cache field, as the TODOs asked for on the next disk-format version bump. Co-Authored-By: Claude Opus 5.5 --- src/governance/governance.cpp | 2 +- src/governance/governance.h | 6 ---- src/test/governance_vote_processing_tests.cpp | 35 +++++++++++++------ 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 6e34c5c56e95..7cb7e6f0ef5e 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -28,7 +28,7 @@ #include -const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16"; +const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-17"; namespace { constexpr std::chrono::seconds GOVERNANCE_DELETION_DELAY{10min}; diff --git a/src/governance/governance.h b/src/governance/governance.h index 33e7996def9a..8cfa0daa189e 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -202,11 +202,8 @@ class GovernanceStore void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs_store) { LOCK(cs_store); - // TODO: Remove the historical invalid-vote-cache field on the next disk-format version bump. - const CacheMap empty_invalid_votes{MAX_CACHE_SIZE}; s << SERIALIZATION_VERSION_STRING << mapErasedGovernanceObjects - << empty_invalid_votes << cmmapOrphanVotes << mapObjects << mapLastMasternodeObject @@ -225,10 +222,7 @@ class GovernanceStore return; } - // TODO: Stop consuming the historical invalid-vote-cache field on the next disk-format version bump. - CacheMap discarded_invalid_votes; s >> mapErasedGovernanceObjects - >> discarded_invalid_votes >> cmmapOrphanVotes >> mapObjects >> mapLastMasternodeObject diff --git a/src/test/governance_vote_processing_tests.cpp b/src/test/governance_vote_processing_tests.cpp index 3317312d477a..d52bdf933bfe 100644 --- a/src/test/governance_vote_processing_tests.cpp +++ b/src/test/governance_vote_processing_tests.cpp @@ -437,7 +437,7 @@ BOOST_AUTO_TEST_CASE(proposal_funding_votes_require_the_voting_key) BOOST_CHECK_EQUAL(stored->GetAbsoluteYesCount(tip_mn_list(), VOTE_SIGNAL_FUNDING), 1); } -BOOST_AUTO_TEST_CASE(legacy_invalid_vote_cache_is_discarded) +BOOST_AUTO_TEST_CASE(legacy_governance_cache_is_discarded) { const uint256 parent_hash{MakeProposal(uint256::ONE).GetHash()}; CKey attacker_key; @@ -449,28 +449,41 @@ BOOST_AUTO_TEST_CASE(legacy_invalid_vote_cache_is_discarded) CacheMap legacy_invalid_votes{3}; legacy_invalid_votes.Insert(forged.GetHash(), forged); const auto proposal = std::make_shared(MakeProposal(uint256::ONE)); + const std::map> objects{{proposal->GetHash(), proposal}}; - CDataStream stream{SER_DISK, CLIENT_VERSION}; - stream << std::string{"CGovernanceManager-Version-16"} + // A version 16 cache, including its invalid-vote field, is dropped as a whole. + CDataStream legacy{SER_DISK, CLIENT_VERSION}; + legacy << std::string{"CGovernanceManager-Version-16"} << std::map{} << legacy_invalid_votes << CacheMultiMap{3} - << std::map>{{proposal->GetHash(), proposal}} + << objects << std::map{} << CDeterministicMNList{}; + TestGovernanceStore legacy_store; + legacy_store.Unserialize(legacy); + BOOST_CHECK_EQUAL(legacy_store.ObjectCount(), 0U); + + // The current format has no invalid-vote field and round-trips. + CDataStream current{SER_DISK, CLIENT_VERSION}; + current << std::string{"CGovernanceManager-Version-17"} + << std::map{} + << CacheMultiMap{3} + << objects + << std::map{} + << CDeterministicMNList{}; + TestGovernanceStore store; - store.Unserialize(stream); + store.Unserialize(current); BOOST_CHECK_EQUAL(store.ObjectCount(), 1U); CDataStream saved{SER_DISK, CLIENT_VERSION}; store.Serialize(saved); - std::string version; - std::map erased_objects; - CacheMap saved_invalid_votes; - saved >> version >> erased_objects >> saved_invalid_votes; - BOOST_CHECK_EQUAL(version, "CGovernanceManager-Version-16"); - BOOST_CHECK_EQUAL(saved_invalid_votes.GetSize(), 0U); + TestGovernanceStore reloaded; + reloaded.Unserialize(saved); + BOOST_CHECK_EQUAL(reloaded.ObjectCount(), 1U); + BOOST_CHECK(saved.empty()); } BOOST_AUTO_TEST_SUITE_END() From 9e5ee1628515e88570ed290392a47c193fefbee9 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 9 Oct 2026 12:47:30 -0500 Subject: [PATCH 2/2] fix: bound governance object signatures Valid governance object signatures are at most 96 bytes (BLS), but the signature field was read without a size limit, so a peer could send an object with a huge signature. Bound the field on every read, network and disk alike; the previous commit discards older caches that could still hold larger values. Co-Authored-By: Claude Opus 5.5 --- doc/release-notes-7831.md | 8 ++++++++ src/governance/common.h | 5 ++++- src/test/governance_vote_wire_tests.cpp | 21 +++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 doc/release-notes-7831.md diff --git a/doc/release-notes-7831.md b/doc/release-notes-7831.md new file mode 100644 index 000000000000..3544e0b87877 --- /dev/null +++ b/doc/release-notes-7831.md @@ -0,0 +1,8 @@ +P2P and network changes +----------------------- + +- Governance object signatures larger than 96 bytes are now rejected, both from + the network and from the on-disk governance cache (`governance.dat`). The + cache format version was bumped for this: on the first start after + upgrading, the existing cache is discarded and governance objects and votes + are synced again from the network. (#7831) diff --git a/src/governance/common.h b/src/governance/common.h index 8272fd5008c9..0078b8572e58 100644 --- a/src/governance/common.h +++ b/src/governance/common.h @@ -10,6 +10,7 @@ #include +#include #include #include @@ -33,6 +34,8 @@ namespace Governance { class Object { public: + static constexpr size_t MAX_SIGNATURE_SIZE{96}; + Object() = default; Object(const uint256& nHashParent, int nRevision, int64_t nTime, const uint256& nCollateralHash, const std::string& strDataHex); @@ -79,7 +82,7 @@ class Object obj.masternodeOutpoint ); if (!(s.GetType() & SER_GETHASH)) { - READWRITE(obj.vchSig); + READWRITE(LIMITED_VECTOR(obj.vchSig, MAX_SIGNATURE_SIZE)); } } }; diff --git a/src/test/governance_vote_wire_tests.cpp b/src/test/governance_vote_wire_tests.cpp index 71bd113fe8e2..42aae0e0e3db 100644 --- a/src/test/governance_vote_wire_tests.cpp +++ b/src/test/governance_vote_wire_tests.cpp @@ -95,6 +95,27 @@ BOOST_AUTO_TEST_CASE(ser_disk_deserialization_unaffected) BOOST_CHECK_EQUAL(ss.size(), 0U); } +// Object signatures are bounded to 96 bytes (BLS) on network and disk reads alike. +BOOST_AUTO_TEST_CASE(object_signature_size_limit) +{ + Governance::Object object; + for (int ser_type : {SER_NETWORK, SER_DISK}) { + for (size_t sig_len : {size_t{0}, size_t{65}, size_t{96}, size_t{97}}) { + object.vchSig.assign(sig_len, 0xAA); + CDataStream ss(ser_type, PROTOCOL_VERSION); + ss << object; + + Governance::Object decoded; + if (sig_len <= Governance::Object::MAX_SIGNATURE_SIZE) { + BOOST_CHECK_NO_THROW(ss >> decoded); + BOOST_CHECK(decoded.vchSig == object.vchSig); + } else { + BOOST_CHECK_THROW(ss >> decoded, std::ios_base::failure); + } + } + } +} + BOOST_AUTO_TEST_CASE(extreme_timestamp_is_rejected_without_overflow) { CGovernanceVote vote;