From 8f0b813fb60378e98fc5d557cc38cfa79dd1f5f6 Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Thu, 9 Jul 2026 21:05:07 -0500 Subject: [PATCH] fix(governance): bound vote signature deserialization Limit network governance-vote signature reads to 96 bytes before allocation and require the structurally valid 65-byte ECDSA or 96-byte BLS encodings. Score malformed or truncated governance vote messages with 100 misbehavior points while preserving disk, hash, and outbound serialization behavior. Add focused unit coverage for malformed, oversized, and valid signature encodings. --- src/Makefile.test.include | 1 + src/governance/net_governance.cpp | 10 ++- src/governance/vote.cpp | 4 ++ src/governance/vote.h | 20 +++++- src/test/governance_vote_wire_tests.cpp | 96 +++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 src/test/governance_vote_wire_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index e2f1782e5d42..e98111bed99c 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -123,6 +123,7 @@ BITCOIN_TESTS =\ test/governance_inv_tests.cpp \ test/governance_superblock_tests.cpp \ test/governance_validators_tests.cpp \ + test/governance_vote_wire_tests.cpp \ test/coinjoin_inouts_tests.cpp \ test/coinjoin_dstxmanager_tests.cpp \ test/coinjoin_queue_tests.cpp \ diff --git a/src/governance/net_governance.cpp b/src/governance/net_governance.cpp index da27cf57b453..b6cf74e1baa5 100644 --- a/src/governance/net_governance.cpp +++ b/src/governance/net_governance.cpp @@ -193,7 +193,15 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa // A NEW GOVERNANCE OBJECT VOTE HAS ARRIVED else if (msg_type == NetMsgType::MNGOVERNANCEOBJECTVOTE) { CGovernanceVote vote; - vRecv >> vote; + // Catch malformed/truncated votes locally so the wire-cap rejection + // in CGovernanceVote scores the peer instead of falling through to + // the outer log-only handler. + try { + vRecv >> vote; + } catch (const std::ios_base::failure&) { + m_peer_manager->PeerMisbehaving(peer.GetId(), 100, "malformed governance vote"); + return; + } uint256 nHash = vote.GetHash(); diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index bcb03a09c75c..91556dbfd4a5 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -9,12 +9,16 @@ #include #include #include +#include #include #include #include #include +static_assert(CGovernanceVote::COMPACT_SIG_SIZE == CPubKey::COMPACT_SIGNATURE_SIZE); +static_assert(CGovernanceVote::BLS_SIG_SIZE == CBLSSignature::SerSize); + std::string CGovernanceVoting::ConvertOutcomeToString(vote_outcome_enum_t nOutcome) { static const std::map mapOutcomeString = { diff --git a/src/governance/vote.h b/src/governance/vote.h index d5e5d96aaedb..876e3c0067dd 100644 --- a/src/governance/vote.h +++ b/src/governance/vote.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -60,6 +61,13 @@ class CGovernanceVote friend bool operator<(const CGovernanceVote& vote1, const CGovernanceVote& vote2); +public: + // Wire-valid signature encodings: compact ECDSA voting key (FUNDING) + // or BLS operator key (other signals). Kept in sync via static_assert + // against CPubKey::COMPACT_SIGNATURE_SIZE / CBLSSignature::SerSize in vote.cpp. + static constexpr size_t COMPACT_SIG_SIZE = 65; + static constexpr size_t BLS_SIG_SIZE = 96; + private: COutPoint masternodeOutpoint; uint256 nParentHash; @@ -123,7 +131,17 @@ class CGovernanceVote { READWRITE(obj.masternodeOutpoint, obj.nParentHash, obj.nVoteOutcome, obj.nVoteSignal, obj.nTime); if (!(s.GetType() & SER_GETHASH)) { - READWRITE(obj.vchSig); + // Network reads: cap the signature vector before allocation and require + // one of the two legitimate encodings. Other paths (disk, hash, write) + // keep the unbounded default. + if (ser_action.ForRead() && (s.GetType() & SER_NETWORK)) { + READWRITE(LIMITED_VECTOR(obj.vchSig, BLS_SIG_SIZE)); + if (obj.vchSig.size() != COMPACT_SIG_SIZE && obj.vchSig.size() != BLS_SIG_SIZE) { + throw std::ios_base::failure("bad governance vote signature size"); + } + } else { + READWRITE(obj.vchSig); + } } SER_READ(obj, obj.UpdateHash()); } diff --git a/src/test/governance_vote_wire_tests.cpp b/src/test/governance_vote_wire_tests.cpp new file mode 100644 index 000000000000..d2a164f67025 --- /dev/null +++ b/src/test/governance_vote_wire_tests.cpp @@ -0,0 +1,96 @@ +// Copyright (c) 2026 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include + +#include + +#include + +#include +#include +#include + +BOOST_FIXTURE_TEST_SUITE(governance_vote_wire_tests, BasicTestingSetup) + +namespace { +void WriteVoteHeader(CDataStream& ss) +{ + ss << COutPoint{uint256::ONE, 0} << uint256::ONE + << int{1} /*outcome*/ << int{1} /*signal*/ << int64_t{1'700'000'000}; +} + +CDataStream MakeVoteWire(size_t sig_len) +{ + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + WriteVoteHeader(ss); + ss << std::vector(sig_len, 0xAA); + return ss; +} +} // namespace + +// Reject invalid signature lengths, including a maximal CompactSize prefix. +BOOST_AUTO_TEST_CASE(rejects_invalid_sizes) +{ + for (size_t bad : {size_t{0}, size_t{64}, size_t{66}, size_t{95}, size_t{97}, size_t{128}}) { + CDataStream ss = MakeVoteWire(bad); + CGovernanceVote vote; + BOOST_CHECK_THROW(ss >> vote, std::ios_base::failure); + } + + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + WriteVoteHeader(ss); + WriteCompactSize(ss, std::numeric_limits::max()); + CGovernanceVote vote; + BOOST_CHECK_THROW(ss >> vote, std::ios_base::failure); +} + +// Truncated element bytes must surface as ios_base::failure so the govobjvote +// handler scores the peer. +BOOST_AUTO_TEST_CASE(truncated_signature_throws_ios_failure) +{ + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + WriteVoteHeader(ss); + ss << uint8_t{CGovernanceVote::BLS_SIG_SIZE}; + ss.write(MakeByteSpan(std::vector(10, 0xBB))); + + CGovernanceVote vote; + BOOST_CHECK_THROW(ss >> vote, std::ios_base::failure); +} + +// 65-byte ECDSA and 96-byte BLS round-trip cleanly over the network. +BOOST_AUTO_TEST_CASE(accepts_legitimate_boundary_sizes) +{ + for (size_t sig_len : {CGovernanceVote::COMPACT_SIG_SIZE, CGovernanceVote::BLS_SIG_SIZE}) { + CDataStream ss = MakeVoteWire(sig_len); + const size_t wire_bytes = ss.size(); + + CGovernanceVote vote; + BOOST_REQUIRE_NO_THROW(ss >> vote); + BOOST_CHECK_EQUAL(ss.size(), 0U); + + CDataStream out(SER_NETWORK, PROTOCOL_VERSION); + out << vote; + BOOST_CHECK_EQUAL(out.size(), wire_bytes); + } +} + +// SER_DISK reads stay unbounded — existing on-disk data must load unchanged. +BOOST_AUTO_TEST_CASE(ser_disk_deserialization_unaffected) +{ + CDataStream ss(SER_DISK, PROTOCOL_VERSION); + WriteVoteHeader(ss); + ss << std::vector(128, 0xCD); + + CGovernanceVote vote; + BOOST_REQUIRE_NO_THROW(ss >> vote); + BOOST_CHECK_EQUAL(ss.size(), 0U); +} + +BOOST_AUTO_TEST_SUITE_END()