Skip to content

Commit fbe37b6

Browse files
molty365claude
andcommitted
Harden BLS weighted-sum/pairing length validation and QC pending-bitset checks
Two robustness fixes in the Savanna consensus path, found during a security review. crypto.cpp: bls_g1_weighted_sum / bls_g2_weighted_sum / bls_pairing validated the element-count-derived span lengths using 32-bit arithmetic, which can overflow for large element counts and let malformed inputs pass the size check before allocation. Compute the expected span lengths in 64-bit so the comparison is exact and malformed calls stay on the return_code::failure path. qc.cpp: validate the pending finalizer-policy signature's vote format and weights before comparing dual-policy finalizer votes, and replace the debug-only assert()s in qc_sig_t::vote_same_at() (compiled out under NDEBUG) with runtime EOS_ASSERT bounds checks on both signatures' vote indices, so a malformed pending vote bitset is rejected rather than reaching unchecked dynamic_bitset indexing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fcb9f5f commit fbe37b6

2 files changed

Lines changed: 22 additions & 8 deletions

File tree

libraries/chain/qc.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,15 @@ void qc_t::verify_basic(const finalizer_policies_t& policies) const {
7171
EOS_ASSERT(policies.pending_finalizer_policy, invalid_qc,
7272
"qc ${bn} contains pending policy signature for nonexistent pending finalizer policy", ("bn", block_num));
7373

74-
// verify that every finalizer included in both policies voted the same
75-
verify_dual_finalizers_votes(policies, active_policy_sig, *pending_policy_sig, block_num);
76-
74+
// Validate the pending signature's vote format and weights BEFORE comparing dual finalizers.
75+
// verify_dual_finalizers_votes() -> vote_same_at() indexes the pending vote bitset, so its
76+
// size must be validated first; otherwise a malformed pending bitset could reach unchecked
77+
// indexing instead of being rejected here.
7778
pending_policy_sig->verify_vote_format(policies.pending_finalizer_policy);
7879
pending_policy_sig->verify_weights(policies.pending_finalizer_policy);
80+
81+
// verify that every finalizer included in both policies voted the same
82+
verify_dual_finalizers_votes(policies, active_policy_sig, *pending_policy_sig, block_num);
7983
} else {
8084
EOS_ASSERT(!policies.pending_finalizer_policy, invalid_qc,
8185
"qc ${bn} does not contain pending policy signature for pending finalizer policy", ("bn", block_num));
@@ -84,8 +88,13 @@ void qc_t::verify_basic(const finalizer_policies_t& policies) const {
8488

8589
// returns true iff the other and I voted in the same way.
8690
bool qc_sig_t::vote_same_at(const qc_sig_t& other, uint32_t my_vote_index, uint32_t other_vote_index) const {
87-
assert(!strong_votes || my_vote_index < strong_votes->size());
88-
assert(!weak_votes || my_vote_index < weak_votes->size());
91+
// Runtime bounds checks for BOTH signatures' vote indices. The previous assert()s are compiled
92+
// out under NDEBUG and only guarded `my_vote_index`; validate `other`'s index too so a
93+
// malformed/undersized bitset is rejected rather than reaching unchecked dynamic_bitset access.
94+
EOS_ASSERT(!strong_votes || my_vote_index < strong_votes->size(), invalid_qc, "qc active strong_votes index out of range");
95+
EOS_ASSERT(!weak_votes || my_vote_index < weak_votes->size(), invalid_qc, "qc active weak_votes index out of range");
96+
EOS_ASSERT(!other.strong_votes || other_vote_index < other.strong_votes->size(), invalid_qc, "qc pending strong_votes index out of range");
97+
EOS_ASSERT(!other.weak_votes || other_vote_index < other.weak_votes->size(), invalid_qc, "qc pending weak_votes index out of range");
8998

9099
// We have already verified the same index has not voted both strong
91100
// and weak for a given qc_sig_t (I or other).

libraries/chain/webassembly/crypto.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,10 @@ namespace eosio::chain::webassembly {
285285
}
286286

287287
int32_t interface::bls_g1_weighted_sum(span<const char> points, span<const char> scalars, const uint32_t n, span<char> result) const {
288-
if(n == 0 || points.size() != n*96 || scalars.size() != n*32 || result.size() != 96)
288+
// Compute the expected span lengths in 64-bit. `n` is uint32 and the products were previously
289+
// computed in 32-bit, which can overflow for large `n` and let malformed inputs pass this
290+
// length check before allocation. 64-bit math keeps the comparison exact.
291+
if(n == 0 || points.size() != static_cast<uint64_t>(n)*96u || scalars.size() != static_cast<uint64_t>(n)*32u || result.size() != 96)
289292
return return_code::failure;
290293

291294
// Use much efficient scale for the special case of n == 1.
@@ -320,7 +323,8 @@ namespace eosio::chain::webassembly {
320323
}
321324

322325
int32_t interface::bls_g2_weighted_sum(span<const char> points, span<const char> scalars, const uint32_t n, span<char> result) const {
323-
if(n == 0 || points.size() != n*192 || scalars.size() != n*32 || result.size() != 192)
326+
// 64-bit length math to avoid the 32-bit n*192 / n*32 wraparound (see bls_g1_weighted_sum).
327+
if(n == 0 || points.size() != static_cast<uint64_t>(n)*192u || scalars.size() != static_cast<uint64_t>(n)*32u || result.size() != 192)
324328
return return_code::failure;
325329

326330
// Use much efficient scale for the special case of n == 1.
@@ -355,7 +359,8 @@ namespace eosio::chain::webassembly {
355359
}
356360

357361
int32_t interface::bls_pairing(span<const char> g1_points, span<const char> g2_points, const uint32_t n, span<char> result) const {
358-
if(n == 0 || g1_points.size() != n*96 || g2_points.size() != n*192 || result.size() != 576)
362+
// 64-bit length math to avoid the 32-bit n*96 / n*192 wraparound (see bls_g1_weighted_sum).
363+
if(n == 0 || g1_points.size() != static_cast<uint64_t>(n)*96u || g2_points.size() != static_cast<uint64_t>(n)*192u || result.size() != 576)
359364
return return_code::failure;
360365
std::vector<std::tuple<bls12_381::g1, bls12_381::g2>> v;
361366
v.reserve(n);

0 commit comments

Comments
 (0)