[ALPHANET] Confidential Voting - #36
Conversation
|
No failures yet, but checks are still running: |
![sentinel-ai-reviewer[bot]](https://avatars.githubusercontent.com/in/320396?s=60&v=4){kind=small}
| Slice const& proof, | ||
| uint256 const& contextHash); | ||
|
|
||
| } // namespace xrpl |
There was a problem hiding this comment.
The aggregated Bulletproof proof slice is passed directly to mpt_verify_aggregated_bulletproof with no size validation — not even an upper bound. BallotCastVote preflight only rejects an empty blob; a multi-megabyte crafted proof passes preflight, reaches preclaim, and forces the Bulletproof verifier to process an arbitrarily large input. Every other verifier in this file (verifySchnorrProof, verifyClawbackProof, verifySendProof) enforces an exact expected byte length before the C call — this one is the only exception. Add an upper-bound check in preflight tied to the maximum option count (8 options), and add a corresponding lower-bound check in verifyBallotRangeProof itself, matching the pattern of every sibling function.
Suggested fix
In BallotCastVote::preflight, replace the bare !ctx.tx[sfZKProof].empty() check with a range check: proof size must be within [minBulletproofSize(n), maxBulletproofSize(n)] where n comes from the ballot's sfOptionCount. In verifyBallotRangeProof itself, add if (proof.size() < kMinAggregatedBulletproofLength || proof.size() > kMaxAggregatedBulletproofLength) return tecBAD_PROOF; before the mpt_verify_aggregated_bulletproof call, consistent with all other verifiers in this file.
|
No failures yet, but checks are still running: |
2 similar comments
|
No failures yet, but checks are still running: |
|
No failures yet, but checks are still running: |
| * @param pubKeys The n mirror public keys, tally key first (33 bytes each). | ||
| * @param c1 The shared ElGamal C1 component (33 bytes). | ||
| * @param c2PerKey The n ElGamal C2 components, one per mirror key (33 bytes). | ||
| * @param commitment The option's Pedersen commitment PC_m (33 bytes). |
There was a problem hiding this comment.
The function iterates over commitments to build a pointer array but never guards against an empty vector before calling mpt_verify_aggregated_bulletproof with a count of zero and a potentially null ptrs.data(). Whether a zero-commitment call returns success or crashes is entirely delegated to an external library whose behavior on that input is unspecified in this codebase. Every other verify* function in this file — verifyBallotVoteLinkage, verifyConvertBackProof, verifySendProof — explicitly rejects empty or out-of-bounds collection inputs before touching the crypto layer; verifyBallotRangeProof is the sole exception. Current callers are protected by an upstream preflight check, but the function itself violates the codebase's established defense-in-depth convention and would silently misbehave if called from any future ballot transaction type without the same upstream guard. Add if (commitments.empty()) return tecINTERNAL; at the top of the function.
Suggested fix
Add if (commitments.empty()) return tecINTERNAL; as the first statement in verifyBallotRangeProof, before the loop that builds ptrs. This is consistent with how verifyBallotVoteLinkage handles n == 0 at its own entry point.
| * | ||
| * @param proof The serialized aggregated Bulletproof. | ||
| * @param commitments One 33-byte Pedersen commitment per option. | ||
| * @param contextHash The 256-bit context hash binding the proof. |
There was a problem hiding this comment.
The transaction-level sfZKProof blob is only checked for non-emptiness in BallotCastVote::preflight, then passed unchanged to verifyBallotRangeProof, which forwards it directly to the external FFI mpt_verify_aggregated_bulletproof with zero size validation. Every other confidential-transfer proof path has a dedicated exact-length constant (kEcSendProofLength, kEcConvertBackProofLength, kEcClawbackProofLength) and a hard exact-size check in preflight — this ballot range proof has neither. The transaction size ceiling of 1 MiB means an attacker can submit a near-megabyte sfZKProof paired with a valid sfEncryptedVotes array; that blob reaches the opaque external library during preclaim with no application-layer bounds check, and if the library dereferences past its expected proof boundary the validator crashes.
Suggested fix
Define a size constant for the aggregated Bulletproof of N commitments (analogous to kEcSendProofLength) — the proof size is deterministic based on N (the Bulletproof size formula gives 32*(2*ceil(log2(N))+6) + fixed overhead bytes). Add an exact-length check in BallotCastVote::preflight that requires sfZKProof.size() == expectedSize(sfEncryptedVotes.size()), and/or add an explicit bounds check at the top of verifyBallotRangeProof before the FFI call.
|
No failures yet, but checks are still running: |
2 similar comments
|
No failures yet, but checks are still running: |
|
No failures yet, but checks are still running: |
dangell7
changed the title
4c2ef3d to
adaae94
Compare
dangell7
added
the
merge-order-07
1 participant
Adds
featureConfidentialVoting.Spec: https://gist.github.com/dangell7/bfaec9c9d0392cb13f27baca862c398a