From 32e77040cedca8b9eb77f241c512fd47a41fe06a Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:19:04 +0000 Subject: [PATCH 1/2] fix: validate subgroup membership in public-key deserialization 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 --- src/ibe/boyen_waters.rs | 43 +++++++++++++++++++++------------ src/ibe/cgw.rs | 40 +++++++++++++++++++------------ src/kem/cgw_kv.rs | 48 ++++++++++++++++++++++--------------- src/kem/kiltz_vahlis_one.rs | 37 +++++++++++++++++++--------- src/util.rs | 13 ++++++++++ 5 files changed, 121 insertions(+), 60 deletions(-) diff --git a/src/ibe/boyen_waters.rs b/src/ibe/boyen_waters.rs index 4a69a35..7101795 100644 --- a/src/ibe/boyen_waters.rs +++ b/src/ibe/boyen_waters.rs @@ -294,21 +294,22 @@ impl Compress for PublicKey { bytes, GT_BYTES, G1_BYTES, G1_BYTES, G2_BYTES, G2_BYTES, G1_BYTES, G1_BYTES, G1_BYTES, G1_BYTES ]; - // from_compressed_unchecked doesn't check whether the element has - // a cofactor. To mount an attack using a cofactor an attacker - // must be able to manipulate the public parameters. But then the - // attacker can simply use parameters they generated themselves. - // Thus checking for a cofactor is superfluous. - - let omega = Gt::from_compressed_unchecked(omega); - let g0 = G1Affine::from_compressed_unchecked(g0); - let g1 = G1Affine::from_compressed_unchecked(g1); - let h0 = G2Affine::from_compressed_unchecked(h0); - let h1 = G2Affine::from_compressed_unchecked(h1); - let v1 = G1Affine::from_compressed_unchecked(v1); - let v2 = G1Affine::from_compressed_unchecked(v2); - let v3 = G1Affine::from_compressed_unchecked(v3); - let v4 = G1Affine::from_compressed_unchecked(v4); + // Use the checked `from_compressed` variant, which verifies that each + // point is in the correct prime-order subgroup. Public keys may be + // deserialized from an unauthenticated source before their integrity is + // verified out-of-band; skipping the subgroup check would let an + // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and + // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). + + let omega = Gt::from_compressed(omega); + let g0 = G1Affine::from_compressed(g0); + let g1 = G1Affine::from_compressed(g1); + let h0 = G2Affine::from_compressed(h0); + let h1 = G2Affine::from_compressed(h1); + let v1 = G1Affine::from_compressed(v1); + let v2 = G1Affine::from_compressed(v2); + let v3 = G1Affine::from_compressed(v3); + let v4 = G1Affine::from_compressed(v4); omega.and_then(|omega| { g0.and_then(|g0| { @@ -491,6 +492,18 @@ impl Compress for CipherText { mod tests { test_ibe!(BoyenWaters); + // Regression test for GHSA-25fp-2fjj-g84w. A public key whose first G1 + // component (g0) encodes a valid on-curve point that lies outside the + // prime-order subgroup must be rejected by the checked `from_bytes`. + #[test] + fn from_bytes_rejects_non_subgroup_point() { + let pk = perform_default().pk; + let mut bytes = pk.to_bytes(); + // Layout: omega (Gt) precedes g0 (first G1 component). + bytes[GT_BYTES..GT_BYTES + G1_BYTES].copy_from_slice(&NON_SUBGROUP_G1_COMPRESSED); + assert!(bool::from(PublicKey::from_bytes(&bytes).is_none())); + } + #[cfg(feature = "zeroize")] #[test] fn shared_secret_zeroize() { diff --git a/src/ibe/cgw.rs b/src/ibe/cgw.rs index 0a1547c..47cbcae 100644 --- a/src/ibe/cgw.rs +++ b/src/ibe/cgw.rs @@ -261,11 +261,12 @@ impl Compress for PublicKey { } fn from_bytes(bytes: &[u8; PK_BYTES]) -> CtOption { - // from_compressed_unchecked doesn't check whether the element has - // a cofactor. To mount an attack using a cofactor an attacker - // must be able to manipulate the public parameters. But then the - // attacker can simply use parameters they generated themselves. - // Thus checking for a cofactor is superfluous. + // Use the checked `from_compressed` variant, which verifies that each + // point is in the correct prime-order subgroup. Public keys may be + // deserialized from an unauthenticated source before their integrity is + // verified out-of-band; skipping the subgroup check would let an + // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and + // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). let mut a_1 = [G1Affine::default(); 2]; let mut w0ta_1 = [G1Affine::default(); 2]; let mut w1ta_1 = [G1Affine::default(); 2]; @@ -275,19 +276,17 @@ impl Compress for PublicKey { for i in 0..2 { let x = i * G1_BYTES; let y = x + G1_BYTES; - is_some &= G1Affine::from_compressed_unchecked(bytes[x..y].try_into().unwrap()) + is_some &= G1Affine::from_compressed(bytes[x..y].try_into().unwrap()) .map(|el| a_1[i] = el) .is_some(); - is_some &= - G1Affine::from_compressed_unchecked(bytes[96 + x..96 + y].try_into().unwrap()) - .map(|el| w0ta_1[i] = el) - .is_some(); - is_some &= - G1Affine::from_compressed_unchecked(bytes[192 + x..192 + y].try_into().unwrap()) - .map(|el| w1ta_1[i] = el) - .is_some(); + is_some &= G1Affine::from_compressed(bytes[96 + x..96 + y].try_into().unwrap()) + .map(|el| w0ta_1[i] = el) + .is_some(); + is_some &= G1Affine::from_compressed(bytes[192 + x..192 + y].try_into().unwrap()) + .map(|el| w1ta_1[i] = el) + .is_some(); } - is_some &= Gt::from_compressed_unchecked(bytes[288..].try_into().unwrap()) + is_some &= Gt::from_compressed(bytes[288..].try_into().unwrap()) .map(|el| kta_t = el) .is_some(); @@ -486,4 +485,15 @@ impl ConditionallySelectable for UserSecretKey { #[cfg(test)] mod tests { test_ibe!(CGW); + + // Regression test for GHSA-25fp-2fjj-g84w. A public key whose first G1 + // component encodes a valid on-curve point that lies outside the prime-order + // subgroup must be rejected by the checked `from_bytes` deserialization. + #[test] + fn from_bytes_rejects_non_subgroup_point() { + let pk = perform_default().pk; + let mut bytes = pk.to_bytes(); + bytes[..G1_BYTES].copy_from_slice(&NON_SUBGROUP_G1_COMPRESSED); + assert!(bool::from(PublicKey::from_bytes(&bytes).is_none())); + } } diff --git a/src/kem/cgw_kv.rs b/src/kem/cgw_kv.rs index 3a1b880..e4f52a9 100644 --- a/src/kem/cgw_kv.rs +++ b/src/kem/cgw_kv.rs @@ -296,11 +296,12 @@ impl Compress for PublicKey { } fn from_bytes(bytes: &[u8; PK_BYTES]) -> CtOption { - // from_compressed_unchecked doesn't check whether the element has - // a cofactor. To mount an attack using a cofactor an attacker - // must be able to manipulate the public parameters. But then the - // attacker can simply use parameters they generated themselves. - // Thus checking for a cofactor is superfluous. + // Use the checked `from_compressed` variant, which verifies that each + // point is in the correct prime-order subgroup. Public keys may be + // deserialized from an unauthenticated source before their integrity is + // verified out-of-band; skipping the subgroup check would let an + // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and + // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). let mut a_1 = [G1Affine::default(); 2]; let mut w0ta_1 = [G1Affine::default(); 2]; let mut w1ta_1 = [G1Affine::default(); 2]; @@ -311,23 +312,20 @@ impl Compress for PublicKey { for i in 0..2 { let x = i * G1_BYTES; let y = x + G1_BYTES; - is_some &= G1Affine::from_compressed_unchecked(bytes[x..y].try_into().unwrap()) + is_some &= G1Affine::from_compressed(bytes[x..y].try_into().unwrap()) .map(|el| a_1[i] = el) .is_some(); - is_some &= - G1Affine::from_compressed_unchecked(bytes[96 + x..96 + y].try_into().unwrap()) - .map(|el| w0ta_1[i] = el) - .is_some(); - is_some &= - G1Affine::from_compressed_unchecked(bytes[192 + x..192 + y].try_into().unwrap()) - .map(|el| w1ta_1[i] = el) - .is_some(); - is_some &= - G1Affine::from_compressed_unchecked(bytes[288 + x..288 + y].try_into().unwrap()) - .map(|el| wprime_1[i] = el) - .is_some(); + is_some &= G1Affine::from_compressed(bytes[96 + x..96 + y].try_into().unwrap()) + .map(|el| w0ta_1[i] = el) + .is_some(); + is_some &= G1Affine::from_compressed(bytes[192 + x..192 + y].try_into().unwrap()) + .map(|el| w1ta_1[i] = el) + .is_some(); + is_some &= G1Affine::from_compressed(bytes[288 + x..288 + y].try_into().unwrap()) + .map(|el| wprime_1[i] = el) + .is_some(); } - is_some &= Gt::from_compressed_unchecked(bytes[384..672].try_into().unwrap()) + is_some &= Gt::from_compressed(bytes[384..672].try_into().unwrap()) .map(|el| kta_t = el) .is_some(); @@ -533,6 +531,18 @@ mod tests { #[cfg(feature = "mkem")] test_multi_kem!(CGWKV); + // Regression test for GHSA-25fp-2fjj-g84w. A public key whose first G1 + // component encodes a valid on-curve point that lies outside the prime-order + // subgroup must be rejected by the checked `from_bytes` deserialization. + #[test] + fn from_bytes_rejects_non_subgroup_point() { + let mut rng = rand::thread_rng(); + let (pk, _sk) = CGWKV::setup(&mut rng); + let mut bytes = pk.to_bytes(); + bytes[..G1_BYTES].copy_from_slice(&NON_SUBGROUP_G1_COMPRESSED); + assert!(bool::from(PublicKey::from_bytes(&bytes).is_none())); + } + #[cfg(feature = "zeroize")] #[test] fn secret_types_zeroize() { diff --git a/src/kem/kiltz_vahlis_one.rs b/src/kem/kiltz_vahlis_one.rs index da933e9..32439b0 100644 --- a/src/kem/kiltz_vahlis_one.rs +++ b/src/kem/kiltz_vahlis_one.rs @@ -203,8 +203,9 @@ impl HashParameters { let mut res = [G1Affine::default(); N]; let mut is_some = Choice::from(1u8); for (i, slot) in res.iter_mut().enumerate() { - // See comment in PublicKey::from_bytes on cofactor. - is_some &= G1Affine::from_compressed_unchecked(array_ref![bytes, i * 48, 48]) + // See comment in PublicKey::from_bytes: checked variant verifies + // subgroup membership (GHSA-25fp-2fjj-g84w). + is_some &= G1Affine::from_compressed(array_ref![bytes, i * 48, 48]) .map(|s| { *slot = s; }) @@ -263,16 +264,17 @@ impl Compress for PublicKey { fn from_bytes(bytes: &[u8; PK_BYTES]) -> CtOption { let (g, hzero, h, u, z) = array_refs![&bytes, 96, 48, HASH_PARAMETER_SIZE, 48, 288]; - // from_compressed_unchecked doesn't check whether the element has - // a cofactor. To mount an attack using a cofactor an attacker - // must be able to manipulate the public parameters. But then the - // attacker can simply use parameters they generated themselves. - // Thus checking for a cofactor is superfluous. - let g = G2Affine::from_compressed_unchecked(g); - let hzero = G1Affine::from_compressed_unchecked(hzero); + // Use the checked `from_compressed` variant, which verifies that each + // point is in the correct prime-order subgroup. Public keys may be + // deserialized from an unauthenticated source before their integrity is + // verified out-of-band; skipping the subgroup check would let an + // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and + // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). + let g = G2Affine::from_compressed(g); + let hzero = G1Affine::from_compressed(hzero); let h = HashParameters::from_bytes(h); - let u = G1Affine::from_compressed_unchecked(u); - let z = Gt::from_compressed_unchecked(z); + let u = G1Affine::from_compressed(u); + let z = Gt::from_compressed(z); g.and_then(|g| { hzero.and_then(|hzero| { @@ -364,6 +366,19 @@ mod tests { #[cfg(feature = "mkem")] test_multi_kem!(KV1); + // Regression test for GHSA-25fp-2fjj-g84w. A public key whose `hzero` G1 + // component encodes a valid on-curve point that lies outside the prime-order + // subgroup must be rejected by the checked `from_bytes` deserialization. + #[test] + fn from_bytes_rejects_non_subgroup_point() { + let mut rng = rand::thread_rng(); + let (pk, _sk) = KV1::setup(&mut rng); + let mut bytes = pk.to_bytes(); + // Layout: g (G2, 96 bytes) precedes hzero (first G1 component). + bytes[G2_BYTES..G2_BYTES + G1_BYTES].copy_from_slice(&NON_SUBGROUP_G1_COMPRESSED); + assert!(bool::from(PublicKey::from_bytes(&bytes).is_none())); + } + // Two distinct strings whose SHA3-512 digests happen to agree on the 8 bits // that the buggy `bits()` read (one bit each from the last 8 bytes of the // digest). Under the old code, both produce the same curve point — i.e. a diff --git a/src/util.rs b/src/util.rs index ed3cc26..2835db2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -20,6 +20,19 @@ pub(crate) const SCALAR_BYTES: usize = 32; /// Size of the (default) identity buffer. pub(crate) const ID_BYTES: usize = 64; +/// Compressed encoding of a G1 point that is on the curve but **not** in the +/// prime-order subgroup (it has a non-trivial cofactor component). The encoding +/// is well-formed, so `from_compressed_unchecked` accepts it, but the checked +/// `from_compressed` rejects it. Used by the `from_bytes` regression tests that +/// guard against the small-subgroup issue (GHSA-25fp-2fjj-g84w). Derived from +/// the non-torsion-free test point in the underlying curve crate. +#[cfg(test)] +pub(crate) const NON_SUBGROUP_G1_COMPRESSED: [u8; G1_BYTES] = [ + 0x92, 0x8d, 0x48, 0x62, 0xa4, 0x04, 0x39, 0xa6, 0x7f, 0xd7, 0x6a, 0x9c, 0x75, 0x60, 0xe2, 0xff, + 0x15, 0x9e, 0x77, 0x0d, 0xcf, 0x68, 0x8f, 0xf7, 0xb2, 0xdd, 0x16, 0x57, 0x92, 0x54, 0x1c, 0x88, + 0xee, 0x76, 0xc8, 0x2e, 0xb7, 0x7d, 0xd6, 0xe9, 0xe7, 0x2c, 0x89, 0xcb, 0xf1, 0xa5, 0x6a, 0x68, +]; + #[inline(always)] pub fn rand_scalar(rng: &mut R) -> Scalar { Scalar::random(rng) From 42a9e894f118e4196a0ab8a2ac7584123b18622b Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:28:41 +0000 Subject: [PATCH 2/2] docs: correct cofactor description in subgroup-check rationale comments 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 --- src/ibe/boyen_waters.rs | 5 +++-- src/ibe/cgw.rs | 5 +++-- src/kem/cgw_kv.rs | 5 +++-- src/kem/kiltz_vahlis_one.rs | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ibe/boyen_waters.rs b/src/ibe/boyen_waters.rs index 7101795..8853699 100644 --- a/src/ibe/boyen_waters.rs +++ b/src/ibe/boyen_waters.rs @@ -298,8 +298,9 @@ impl Compress for PublicKey { // point is in the correct prime-order subgroup. Public keys may be // deserialized from an unauthenticated source before their integrity is // verified out-of-band; skipping the subgroup check would let an - // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and - // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). + // adversary embed a point outside the prime-order subgroup (BLS12-381 + // has a large, non-trivial cofactor) and mount a small-subgroup attack + // (GHSA-25fp-2fjj-g84w). let omega = Gt::from_compressed(omega); let g0 = G1Affine::from_compressed(g0); diff --git a/src/ibe/cgw.rs b/src/ibe/cgw.rs index 47cbcae..e7850a4 100644 --- a/src/ibe/cgw.rs +++ b/src/ibe/cgw.rs @@ -265,8 +265,9 @@ impl Compress for PublicKey { // point is in the correct prime-order subgroup. Public keys may be // deserialized from an unauthenticated source before their integrity is // verified out-of-band; skipping the subgroup check would let an - // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and - // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). + // adversary embed a point outside the prime-order subgroup (BLS12-381 + // has a large, non-trivial cofactor) and mount a small-subgroup attack + // (GHSA-25fp-2fjj-g84w). let mut a_1 = [G1Affine::default(); 2]; let mut w0ta_1 = [G1Affine::default(); 2]; let mut w1ta_1 = [G1Affine::default(); 2]; diff --git a/src/kem/cgw_kv.rs b/src/kem/cgw_kv.rs index e4f52a9..c4d8e21 100644 --- a/src/kem/cgw_kv.rs +++ b/src/kem/cgw_kv.rs @@ -300,8 +300,9 @@ impl Compress for PublicKey { // point is in the correct prime-order subgroup. Public keys may be // deserialized from an unauthenticated source before their integrity is // verified out-of-band; skipping the subgroup check would let an - // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and - // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). + // adversary embed a point outside the prime-order subgroup (BLS12-381 + // has a large, non-trivial cofactor) and mount a small-subgroup attack + // (GHSA-25fp-2fjj-g84w). let mut a_1 = [G1Affine::default(); 2]; let mut w0ta_1 = [G1Affine::default(); 2]; let mut w1ta_1 = [G1Affine::default(); 2]; diff --git a/src/kem/kiltz_vahlis_one.rs b/src/kem/kiltz_vahlis_one.rs index 32439b0..f266ef8 100644 --- a/src/kem/kiltz_vahlis_one.rs +++ b/src/kem/kiltz_vahlis_one.rs @@ -268,8 +268,9 @@ impl Compress for PublicKey { // point is in the correct prime-order subgroup. Public keys may be // deserialized from an unauthenticated source before their integrity is // verified out-of-band; skipping the subgroup check would let an - // adversary embed low-order points (BLS12-381 G1 cofactor is 76) and - // mount a small-subgroup attack (GHSA-25fp-2fjj-g84w). + // adversary embed a point outside the prime-order subgroup (BLS12-381 + // has a large, non-trivial cofactor) and mount a small-subgroup attack + // (GHSA-25fp-2fjj-g84w). let g = G2Affine::from_compressed(g); let hzero = G1Affine::from_compressed(hzero); let h = HashParameters::from_bytes(h);