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

ensure constraints are applied appropriately #95

Merged
merged 2 commits into from
Oct 11, 2024
Merged

Conversation

Chengxuan
Copy link
Contributor

Resolves #92

The type of failures captured in #92 is actually hard to capture by tests because it requires a circuit with missing constraints. Step to reproduce the issue:

  1. Write a circuit that lacks constraint generation of a signal, generate a verifier
  2. The prover will need to modify the circuit to remove the check that is not protected by any constraint and use it to generate a proof.
  3. The verifier will not catch any violation of the removed check

Signed-off-by: Chengxuan Xing <[email protected]>
@jimthematrix
Copy link
Contributor

The fix looks great @Chengxuan. Can you add tests under zkp/js/integration-test that would simulate an attack against the unsafe circuits like before the fix. Using a procedure like the following, for example with the anon circuit:

  • construct valid private inputs:
    • input1: hash(10, salt1, sender)
    • input1: hash(20, salt2, sender)
    • output1: hash(30, salt3, sender)
  • calculate the witness and generate the proof
  • verify using a modified set of public inputs:
    • replace output1 with hash(100, salt3, sender)

I believe under the circuit before the fix, the verification would succeed, meaning we have allowed any user to give themselves tokens out of thin area. But with the fix it should fail the verification

@Chengxuan
Copy link
Contributor Author

Chengxuan commented Oct 11, 2024

@jimthematrix I've raised a separate PR #96 to address #94 which covered the test case you mentioned above

Copy link
Contributor

@jimthematrix jimthematrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Chengxuan will explain in the issue #92 the details of the vulnerability, which involves modifying the circuit without causing the verification keys to be updated, to allow a proof to be generated from invalid private inputs, but can be successfully verified.

@jimthematrix jimthematrix merged commit e9ff695 into main Oct 11, 2024
6 checks passed
@jimthematrix jimthematrix deleted the constraint-fix branch October 11, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ensure constraints in Circom circuits are generated correctly
2 participants