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/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() 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;