diff --git a/shared/crypto/bn254/lib.zig b/shared/crypto/bn254/lib.zig index d2297b4f04..12fc8a1643 100644 --- a/shared/crypto/bn254/lib.zig +++ b/shared/crypto/bn254/lib.zig @@ -178,18 +178,19 @@ pub const G2 = struct { }; } - fn isWellFormed(p: G2) !void { - // zero-point always well formed, no matter what X and Y are - if (p.isZero()) return; + const Validate = enum { curve_only, curve_and_subgroup }; - // Check that y^2 = x^3 + b + // y^2 == x^3 + b. Callers guard the point at infinity before this. + fn isOnCurve(p: G2) bool { const y2 = p.y.sq(); const x3b = p.x.sq().mul(p.x).add(Fp2.constants.twist_b_mont); - if (!y2.eql(x3b)) return error.NotWellFormed; + return y2.eql(x3b); + } - // G2 does *not* have prime order, so we need to perform a secondary subgroup membership check. - // https://eprint.iacr.org/2022/348, Sec 3.1. - // [r]P == 0 <==> [x+1]P + ψ([x]P) + ψ²([x]P) = ψ³([2x]P) + // G2 has a large cofactor, so an on-curve point need not lie in the prime-order + // (order-r) subgroup. https://eprint.iacr.org/2022/348, Sec 3.1. + // [r]P == 0 <==> [x+1]P + ψ([x]P) + ψ²([x]P) = ψ³([2x]P) + fn isInSubgroup(p: G2) bool { const xp: G2 = shared.mulScalar(p, Fp.constants.x); const psi = xp.frob(); @@ -198,10 +199,10 @@ pub const G2 = struct { const l = shared.addMixed(xp, p).add(psi).add(psi2); const r = shared.dbl(psi2.frob()); - if (!l.eql(r)) return error.NotWellFormed; + return l.eql(r); } - fn fromBytes(input: *const [128]u8, endian: std.builtin.Endian) !G2 { + fn fromBytes(input: *const [128]u8, endian: std.builtin.Endian, validate: Validate) !G2 { var g2: G2 = try .fromBytesInternal(input, endian); if (g2.isZero()) return g2; @@ -209,7 +210,15 @@ pub const G2 = struct { g2.y.toMont(); g2.z = .one; - try g2.isWellFormed(); + if (!g2.isOnCurve()) return error.NotWellFormed; + + // V0 G2 addition is curve-only (SIMD-0302); mul and pairing require subgroup + // membership. Agave gates the same way (into_affine_unchecked vs TryFrom), + // as does Firedancer (g2_frombytes_check_eq_only vs _check_subgroup). + // https://github.com/anza-xyz/solana-sdk/blob/e7db3b9d9f61efcb8fa2547f7371a4be2b6942d7/bn254/src/lib.rs#L241-L261 + if (validate == .curve_and_subgroup and !g2.isInSubgroup()) { + return error.NotWellFormed; + } return g2; } @@ -370,14 +379,14 @@ pub const G2 = struct { } pub fn addSyscall(out: *[128]u8, input: *const [256]u8, endian: std.builtin.Endian) !void { - const x: G2 = try .fromBytes(input[0..128], endian); - const y: G2 = try .fromBytes(input[128..256], endian); + const x: G2 = try .fromBytes(input[0..128], endian, .curve_only); + const y: G2 = try .fromBytes(input[128..256], endian, .curve_only); const result = shared.affineAdd(x, y); result.toBytes(out, endian); } pub fn mulSyscall(out: *[128]u8, input: *const [160]u8, endian: std.builtin.Endian) !void { - const a: G2 = try .fromBytes(input[0..128], endian); + const a: G2 = try .fromBytes(input[0..128], endian, .curve_and_subgroup); const scalar = input[128..][0..32].*; const b: u256 = @bitCast(switch (endian) { .big => Fp.byteSwap(scalar), @@ -551,7 +560,7 @@ pub fn pairingSyscall(out: *[32]u8, input: []const u8, endian: std.builtin.Endia var r: Fp12 = .one; for (0..num_elements) |i| { const a: G1 = try .fromBytes(input[i * 192 ..][0..64], endian); - const b: G2 = try .fromBytes(input[i * 192 ..][64..][0..128], endian); + const b: G2 = try .fromBytes(input[i * 192 ..][64..][0..128], endian, .curve_and_subgroup); // Skip any pair where either A or B are points at infinity. if (a.isZero() or b.isZero()) continue; diff --git a/shared/crypto/bn254/tests.zig b/shared/crypto/bn254/tests.zig index b9cc160068..2102c32d4f 100644 --- a/shared/crypto/bn254/tests.zig +++ b/shared/crypto/bn254/tests.zig @@ -579,6 +579,39 @@ test "edge cases" { } } +// Regression for #1713: ALT_BN128_G2_ADD must accept any on-curve G2 point. G2 has a +// large cofactor, so add decodes curve-only (Agave into_affine_unchecked, Firedancer +// g2_frombytes_check_eq_only); only mul and pairing check subgroup membership. P below is +// on curve but outside the order-r subgroup (the same point test "edge cases" rejects). +test "G2 addition accepts on-curve non-subgroup point" { + const point_hex = + "0000000000000000000000000000000000000000000000000000000000000001" ++ + "0000000000000000000000000000000000000000000000000000000000000000" ++ + "28a7a81c6bf2a75dc9f0125bb581747e9e6b33fc3b2710a2309cef97a3163c65" ++ + "23712136978ed49faf2120ca4f7f71cfd4e7b46ffa0ea89edbc94ddc59238e9f"; + + var point: [128]u8 = undefined; + _ = try std.fmt.hexToBytes(&point, point_hex); + + // Control: multiplication requires subgroup membership and rejects P. + // The scalar is 1, encoded big-endian. + var mul_input: [160]u8 = @splat(0); + @memcpy(mul_input[0..128], &point); + mul_input[159] = 1; + var mul_output: [128]u8 = undefined; + try std.testing.expectError( + error.NotWellFormed, + G2.mulSyscall(&mul_output, &mul_input, .big), + ); + + // V0 addition only requires P to be on curve. P + infinity must return P. + var add_input: [256]u8 = @splat(0); + @memcpy(add_input[0..128], &point); + var add_output: [128]u8 = undefined; + try G2.addSyscall(&add_output, &add_input, .big); + try std.testing.expectEqualSlices(u8, &point, &add_output); +} + fn bswapG1(bytes: *[64]u8) void { const c0: u256 = @bitCast(bytes[0..32].*); const c1: u256 = @bitCast(bytes[32..64].*);