status-im / nim-blscurve

Nim implementation of BLS signature scheme (Boneh-Lynn-Shacham) over Barreto-Lynn-Scott (BLS) curve BLS12-381
Apache License 2.0
26 stars 11 forks source link

Reject invalid serialized infinity points #64

Closed mratsim closed 3 years ago

mratsim commented 4 years ago

From https://github.com/status-im/nim-beacon-chain/issues/555?notification_referrer_id=MDE4Ok5vdGlmaWNhdGlvblRocmVhZDYzMjEwOTE1MzoyMjczODMxNw%3D%3D#issuecomment-555964021

It's an invalid encoding of an infinity point.

The issue is that a signature is represented as a (G1, G1) with G1 using 381 bits out of 48 bytes (384 bits) and some of the extra 3 bits are used as flags according to the Zcash serialization spec: https://github.com/zcash/librustzcash/tree/f55f094e/pairing/src/bls12_381#serialization Note that the Zcash serialization spec is recommended in IETF (https://tools.ietf.org/id/draft-irtf-cfrg-pairing-friendly-curves-07.html#name-zcash-serialization-format-)

image

The Ethereum test generators has a 50% chance of setting this infinity bit to 1. But if it's set to 1, the rest of the bits should be set to 0.

The NBC fix is that this was raised with test data for the SSZ test vectors, for this suite we only need opaque blob, we never need to deserialize the signatures.

The crypto fix is to always reject invalid encoding of infinity points.

In BLST this is done by copying the input bytes, then checking if they were all zeros: https://github.com/supranational/blst/blob/27d3083e/src/e2.c#L300-L307

This is not done in Milagro