semaphore-protocol / semaphore

A zero-knowledge protocol for anonymous interactions.
https://semaphore.pse.dev
MIT License
883 stars 193 forks source link

Constrain babyjubjub secret scalar to be < `l` #744

Closed 0xbok closed 4 months ago

0xbok commented 4 months ago

Describe the bug

Context: semaphore.circom#L44

Following Geometry's bug disclosure (Circom_bug.pdf), it's known that babyjubjub secret key has to be constrained to be < l. l is as defined https://eips.ethereum.org/EIPS/eip-2494 and the same as r defined in Geometry's bug report. circomlib's BabyPbk() circuit (as of at the time of writing this issue) enforces the secret scalar to fit in 253 bits, where as l is of 251 bits.

Fix Constrain the secret scalar to be < l using LessThan template. The fix and the reasoning is explained here: https://hackmd.io/@blockdev/Bkj0Qp8x0