Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing checks to ensure zk proof inputs are less than SNARK_SCALAR_FIELD #90

Closed
kcharbo3 opened this issue Mar 7, 2022 · 0 comments · Fixed by #91
Closed

Missing checks to ensure zk proof inputs are less than SNARK_SCALAR_FIELD #90

kcharbo3 opened this issue Mar 7, 2022 · 0 comments · Fixed by #91
Assignees
Labels
bug 🐛 Something isn't working

Comments

@kcharbo3
Copy link

kcharbo3 commented Mar 7, 2022

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.

@cedoor cedoor self-assigned this Mar 9, 2022
@cedoor cedoor added the bug 🐛 Something isn't working label Mar 9, 2022
@cedoor cedoor linked a pull request Mar 9, 2022 that will close this issue
13 tasks
@cedoor cedoor closed this as completed Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants