Harden BLS host-function length validation and QC pending-bitset checks (Savanna path)#7
Open
TheJudii wants to merge 1 commit into
Open
Conversation
729925e to
224f4da
Compare
…et 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>
224f4da to
fbe37b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A couple of robustness fixes in the Savanna consensus path, from the same review as the contract-side changes in telosnetwork/telos.contracts#64. Targets
release/1.2-teloszero(the ref the contracts repo pins asTELOSZERO_REF). Both come from upstream Antelope/Spring, so they're worth mirroring upstream as well.BLS length validation (
crypto.cpp)bls_g1_weighted_sum,bls_g2_weighted_sum, andbls_pairingwork out the expectedpoints/scalarsspan lengths from the element countnusing 32-bit arithmetic. For a large enoughnthat product can overflow, so the length check isn't exact and a malformed call could slip past it before allocation. Doing the length math in 64-bit keeps the comparison exact and sends bad inputs down the normalreturn_code::failurepath.Pending-QC bitset validation (
qc.cpp)verify_basic()was comparing the dual-policy finalizer votes — which indexes the pending vote bitset — before it had validated that bitset's format, andvote_same_at()only had debugassert()s (compiled out in release, and covering just one of the two indices). Reordered so the pending signature's format and weights are checked first, and replaced theassert()s with realEOS_ASSERTbounds checks on both sides, so a malformed bitset is rejected cleanly rather than indexed blindly.No ABI or wire-format change — this is input-validation hardening. Worth adding regression coverage for a malformed pending bitset (with active and pending policies sharing a finalizer) and an oversized
non the BLS calls. CI here is the build/test gate.