Skip to content

Commit 24c0ead

Browse files
Merge #7440: fix(net): bound governance vote signature deserialization
8f0b813 fix(governance): bound vote signature deserialization (PastaClaw) Pull request description: Uses the shared bounded-vector deserialization primitive merged in #7439. ## Motivation Governance vote signatures were deserialized through the generic byte-vector path. A peer could declare a very large signature length, causing allocation before the stream reported truncation. The outer message-processing catch did not score or disconnect the peer, allowing repeated malformed messages. ## Changes - bound network governance-vote signature reads to 96 bytes before allocation - require one of the two structurally valid encodings: 65-byte compact ECDSA or 96-byte BLS - score malformed or truncated governance vote messages with 100 misbehavior points - preserve disk, hash, and outbound serialization behavior - add focused unit coverage ## Testing - `./src/test/test_dash --run_test=governance_vote_wire_tests` (4/4 tests) - `./src/test/test_dash --run_test=serialize_tests` (10/10 tests) - `test/lint/lint-python.py` - `git diff --check upstream/develop...HEAD` ACKs for top commit: knst: utACK 8f0b813 Tree-SHA512: 5cd804ffb410f47936615d230f5a5ebe4b2b4e1dd85455acb42df37fb65c0397f05c38704900a3ecf5d2729e157d03b298c4a7774b7764b3830cb57273a724cc
2 parents 5260466 + 8f0b813 commit 24c0ead

5 files changed

Lines changed: 129 additions & 2 deletions

File tree

src/Makefile.test.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ BITCOIN_TESTS =\
123123
test/governance_inv_tests.cpp \
124124
test/governance_superblock_tests.cpp \
125125
test/governance_validators_tests.cpp \
126+
test/governance_vote_wire_tests.cpp \
126127
test/coinjoin_inouts_tests.cpp \
127128
test/coinjoin_dstxmanager_tests.cpp \
128129
test/coinjoin_queue_tests.cpp \

src/governance/net_governance.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,15 @@ void NetGovernance::ProcessMessage(CNode& peer, const std::string& msg_type, CDa
193193
// A NEW GOVERNANCE OBJECT VOTE HAS ARRIVED
194194
else if (msg_type == NetMsgType::MNGOVERNANCEOBJECTVOTE) {
195195
CGovernanceVote vote;
196-
vRecv >> vote;
196+
// Catch malformed/truncated votes locally so the wire-cap rejection
197+
// in CGovernanceVote scores the peer instead of falling through to
198+
// the outer log-only handler.
199+
try {
200+
vRecv >> vote;
201+
} catch (const std::ios_base::failure&) {
202+
m_peer_manager->PeerMisbehaving(peer.GetId(), 100, "malformed governance vote");
203+
return;
204+
}
197205

198206
uint256 nHash = vote.GetHash();
199207

src/governance/vote.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@
99
#include <evo/dmn_types.h>
1010
#include <masternode/sync.h>
1111
#include <messagesigner.h>
12+
#include <pubkey.h>
1213

1314
#include <chainparams.h>
1415
#include <logging.h>
1516
#include <timedata.h>
1617
#include <util/string.h>
1718

19+
static_assert(CGovernanceVote::COMPACT_SIG_SIZE == CPubKey::COMPACT_SIGNATURE_SIZE);
20+
static_assert(CGovernanceVote::BLS_SIG_SIZE == CBLSSignature::SerSize);
21+
1822
std::string CGovernanceVoting::ConvertOutcomeToString(vote_outcome_enum_t nOutcome)
1923
{
2024
static const std::map<vote_outcome_enum_t, std::string> mapOutcomeString = {

src/governance/vote.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <hash.h>
99
#include <primitives/transaction.h>
10+
#include <serialize.h>
1011
#include <uint256.h>
1112
#include <util/string.h>
1213

@@ -60,6 +61,13 @@ class CGovernanceVote
6061

6162
friend bool operator<(const CGovernanceVote& vote1, const CGovernanceVote& vote2);
6263

64+
public:
65+
// Wire-valid signature encodings: compact ECDSA voting key (FUNDING)
66+
// or BLS operator key (other signals). Kept in sync via static_assert
67+
// against CPubKey::COMPACT_SIGNATURE_SIZE / CBLSSignature::SerSize in vote.cpp.
68+
static constexpr size_t COMPACT_SIG_SIZE = 65;
69+
static constexpr size_t BLS_SIG_SIZE = 96;
70+
6371
private:
6472
COutPoint masternodeOutpoint;
6573
uint256 nParentHash;
@@ -123,7 +131,17 @@ class CGovernanceVote
123131
{
124132
READWRITE(obj.masternodeOutpoint, obj.nParentHash, obj.nVoteOutcome, obj.nVoteSignal, obj.nTime);
125133
if (!(s.GetType() & SER_GETHASH)) {
126-
READWRITE(obj.vchSig);
134+
// Network reads: cap the signature vector before allocation and require
135+
// one of the two legitimate encodings. Other paths (disk, hash, write)
136+
// keep the unbounded default.
137+
if (ser_action.ForRead() && (s.GetType() & SER_NETWORK)) {
138+
READWRITE(LIMITED_VECTOR(obj.vchSig, BLS_SIG_SIZE));
139+
if (obj.vchSig.size() != COMPACT_SIG_SIZE && obj.vchSig.size() != BLS_SIG_SIZE) {
140+
throw std::ios_base::failure("bad governance vote signature size");
141+
}
142+
} else {
143+
READWRITE(obj.vchSig);
144+
}
127145
}
128146
SER_READ(obj, obj.UpdateHash());
129147
}
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright (c) 2026 The Dash Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <governance/vote.h>
6+
#include <primitives/transaction.h>
7+
#include <serialize.h>
8+
#include <streams.h>
9+
#include <uint256.h>
10+
#include <version.h>
11+
12+
#include <test/util/setup_common.h>
13+
14+
#include <boost/test/unit_test.hpp>
15+
16+
#include <ios>
17+
#include <limits>
18+
#include <vector>
19+
20+
BOOST_FIXTURE_TEST_SUITE(governance_vote_wire_tests, BasicTestingSetup)
21+
22+
namespace {
23+
void WriteVoteHeader(CDataStream& ss)
24+
{
25+
ss << COutPoint{uint256::ONE, 0} << uint256::ONE
26+
<< int{1} /*outcome*/ << int{1} /*signal*/ << int64_t{1'700'000'000};
27+
}
28+
29+
CDataStream MakeVoteWire(size_t sig_len)
30+
{
31+
CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
32+
WriteVoteHeader(ss);
33+
ss << std::vector<unsigned char>(sig_len, 0xAA);
34+
return ss;
35+
}
36+
} // namespace
37+
38+
// Reject invalid signature lengths, including a maximal CompactSize prefix.
39+
BOOST_AUTO_TEST_CASE(rejects_invalid_sizes)
40+
{
41+
for (size_t bad : {size_t{0}, size_t{64}, size_t{66}, size_t{95}, size_t{97}, size_t{128}}) {
42+
CDataStream ss = MakeVoteWire(bad);
43+
CGovernanceVote vote;
44+
BOOST_CHECK_THROW(ss >> vote, std::ios_base::failure);
45+
}
46+
47+
CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
48+
WriteVoteHeader(ss);
49+
WriteCompactSize(ss, std::numeric_limits<uint64_t>::max());
50+
CGovernanceVote vote;
51+
BOOST_CHECK_THROW(ss >> vote, std::ios_base::failure);
52+
}
53+
54+
// Truncated element bytes must surface as ios_base::failure so the govobjvote
55+
// handler scores the peer.
56+
BOOST_AUTO_TEST_CASE(truncated_signature_throws_ios_failure)
57+
{
58+
CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
59+
WriteVoteHeader(ss);
60+
ss << uint8_t{CGovernanceVote::BLS_SIG_SIZE};
61+
ss.write(MakeByteSpan(std::vector<unsigned char>(10, 0xBB)));
62+
63+
CGovernanceVote vote;
64+
BOOST_CHECK_THROW(ss >> vote, std::ios_base::failure);
65+
}
66+
67+
// 65-byte ECDSA and 96-byte BLS round-trip cleanly over the network.
68+
BOOST_AUTO_TEST_CASE(accepts_legitimate_boundary_sizes)
69+
{
70+
for (size_t sig_len : {CGovernanceVote::COMPACT_SIG_SIZE, CGovernanceVote::BLS_SIG_SIZE}) {
71+
CDataStream ss = MakeVoteWire(sig_len);
72+
const size_t wire_bytes = ss.size();
73+
74+
CGovernanceVote vote;
75+
BOOST_REQUIRE_NO_THROW(ss >> vote);
76+
BOOST_CHECK_EQUAL(ss.size(), 0U);
77+
78+
CDataStream out(SER_NETWORK, PROTOCOL_VERSION);
79+
out << vote;
80+
BOOST_CHECK_EQUAL(out.size(), wire_bytes);
81+
}
82+
}
83+
84+
// SER_DISK reads stay unbounded — existing on-disk data must load unchanged.
85+
BOOST_AUTO_TEST_CASE(ser_disk_deserialization_unaffected)
86+
{
87+
CDataStream ss(SER_DISK, PROTOCOL_VERSION);
88+
WriteVoteHeader(ss);
89+
ss << std::vector<unsigned char>(128, 0xCD);
90+
91+
CGovernanceVote vote;
92+
BOOST_REQUIRE_NO_THROW(ss >> vote);
93+
BOOST_CHECK_EQUAL(ss.size(), 0U);
94+
}
95+
96+
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)