semaphore-protocol/semaphore

Missing checks to ensure zk proof inputs are less than SNARK_SCALAR_FIELD

kcharbo3 opened this issue · 0 comments

Description
The SemaphoreGroup.sol contract uses the "groupId" as the external nullifier. Therefore, the groupId will be provided as a public input when calling Verifier.verifyProof. When a user creates a new poll via SemaphoreVoting.createPoll, the user can enter any uint256 value as the poll id, which will then be the group Id. The issue arises when the user inputs a value for pollId that is greater than the SNARK_SCALAR_FIELD constant. Then, the call to verify any proofs for this poll will fail because the Verifier checks:
require(input[i] < snark_scalar_field, "verifier-gte-snark-scalar-field");
https://github.com/appliedzkp/semaphore/blob/main/contracts/base/Verifier.sol#L285
for each public input.

Since both the SemaphoreVoting.sol and SemaphoreWhistleblowing.sol contracts use the groupId as external nullifiers, this issue is present in both.

Possible Fix
Add a constraint in the SemaphoreGroup.sol contract to check that the groupId is less than the SNARK_SCALAR_FIELD. This will protect any extensions that use the group id as an external nullifier.