fix: validate subgroup membership in public-key deserialization (GHSA-25fp-2fjj-g84w)#51
Open
dobby-coder[bot] wants to merge 2 commits into
Open
fix: validate subgroup membership in public-key deserialization (GHSA-25fp-2fjj-g84w)#51dobby-coder[bot] wants to merge 2 commits into
dobby-coder[bot] wants to merge 2 commits into
Conversation
Several `PublicKey::from_bytes` implementations (and KV1's `HashParameters::from_bytes`) deserialized compressed curve points with `from_compressed_unchecked`, which skips the prime-order subgroup check. A public key received from an unauthenticated source could embed a valid on-curve point outside the prime-order subgroup (BLS12-381 G1 cofactor is 76). Using such a key during encapsulation/encryption could draw session keys from a small subgroup, breaking IND-CCA2 guarantees. Switch to the checked `from_compressed` variant in the public-key `from_bytes` implementations for CGW, CGW-KV, KV1 and Boyen-Waters, and update the now-obsolete justification comments. Adds a regression test per scheme that feeds a well-formed but non-subgroup G1 point and asserts `from_bytes` rejects it. Fixes GHSA-25fp-2fjj-g84w Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The subgroup-check rationale comments stated "BLS12-381 G1 cofactor is 76", but the actual G1 cofactor is a 126-bit value (~7.6e37). The stray "76" massively understated the attack surface in a security-facing comment. Reworded to describe the large, non-trivial cofactor without a wrong number, matching the phrasing already used in util.rs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
There was a problem hiding this comment.
Rules Dobby 2 — gatekeeper sign-off (posted as COMMENT only because GitHub blocks APPROVE/REQUEST_CHANGES on a self-authored PR; treat this as an approval).
Ran the full rule check against the memory ruleset and merged with Review Dobby 2's findings.
- Rules: compliant — conventional-commit title ✅, tests present (per-scheme rejection-path regression tests) ✅,
cargo fmt/build/tests green ✅ (31/31, all features), draft PR linked from the advisory ✅. - Review Dobby 2: 0 blocking bugs; one nit — the security-rationale comment claimed "BLS12-381 G1 cofactor is 76" in 4 files (the real G1 cofactor is a 126-bit value), and the same wrong number appeared in the PR body.
Resolved inline rather than looping (nit + trivially self-resolvable on a self-authored PR): pushed 42a9e89 correcting all four comments to describe the large, non-trivial cofactor without a wrong figure (matching util.rs), and fixed the PR body. Re-ran fmt + full suite: clean, 31/31 pass.
No outstanding findings — good to go for maintainer review.
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.
Summary
Fixes the medium-severity finding in GHSA-25fp-2fjj-g84w (tracked in #50): several public-key
from_bytesimplementations deserialized compressed curve points withfrom_compressed_unchecked, which skips the prime-order subgroup / cofactor check.A public key received from an unauthenticated source (e.g. over a network, before integrity verification) could embed a valid on-curve point that lies outside the prime-order subgroup (BLS12-381 has a large, non-trivial cofactor). Using such a malformed key during encapsulation/encryption can draw session keys from a small subgroup, breaking IND-CCA2 guarantees. The previous inline comments justified the unchecked variant on a "trusted infrastructure" assumption that the API did not actually enforce at the boundary.
Changes
Switched to the checked
from_compressedvariant in every public-keyfrom_byteslisted in the advisory, and updated the now-obsolete justification comments:src/ibe/cgw.rs—PublicKey::from_bytessrc/kem/cgw_kv.rs—PublicKey::from_bytessrc/kem/kiltz_vahlis_one.rs—PublicKey::from_bytesandHashParameters::from_bytessrc/ibe/boyen_waters.rs—PublicKey::from_bytesUser-secret-key and ciphertext
from_bytesalready used the checked variant, so they are unchanged.Tests
Added a per-scheme regression test (
from_bytes_rejects_non_subgroup_point) that overwrites a G1 component of a valid serialized public key with a well-formed but non-subgroup G1 point and assertsfrom_bytesreturnsCtOption::none. The test vector is a compressed encoding of an on-curve, non-torsion-free point (from_compressed_uncheckedaccepts it;from_compressedrejects it), stored once asNON_SUBGROUP_G1_COMPRESSEDinsrc/util.rs.I confirmed the tests genuinely catch the vulnerability: temporarily reverting a single call back to
from_compressed_uncheckedmakes the corresponding test fail.Verification
cargo test --release --all-features— 31 passed, 0 failedcargo clippy --all-features --all-targets— cleancargo fmt -- --check— cleanAdvisory: https://github.com/encryption4all/ibe/security/advisories/GHSA-25fp-2fjj-g84w
Closes #50
🤖 Generated with Claude Code