Abort extracting vk when there are no phase 2 contributions#620
Abort extracting vk when there are no phase 2 contributions#620StefanosChaliasos wants to merge 2 commits intoiden3:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a soundness guard to prevent exporting/using Groth16 verification keys when the zkey appears to have no Phase 2 contribution (heuristic: vk_gamma_2 === vk_delta_2), addressing a known setup exploit class.
Changes:
- Abort Groth16 VK export if
vk_gamma_2equalsvk_delta_2. - Add the same guard to Solidity verifier export.
- Make Groth16 verification return
false(with an error log) when the VK is detected as insecure.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/zkey_export_verificationkey.js | Adds a hard failure during Groth16 VK export when vk_gamma_2 === vk_delta_2. |
| src/zkey_export_solidityverifier.js | Adds a Groth16 VK sanity check before rendering the Solidity verifier. |
| src/groth16_verify.js | Rejects verification keys with vk_gamma_2 === vk_delta_2 during Groth16 verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (curve.G2.eq(vk_gamma_2, vk_delta_2)) { | ||
| if (logger) logger.error("SOUNDNESS ERROR: vk_gamma_2 and vk_delta_2 are equal. This verification key is insecure, the zkey likely has no phase 2 contribution. Proofs can be forged. Aborting verification."); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
New behavior returns false when vk_gamma_2 equals vk_delta_2, but there doesn’t appear to be a test exercising this soundness guard. Adding a unit/integration test that verifies groth16.verify rejects an insecure verification key (e.g., by crafting a VK with vk_delta_2 copied from vk_gamma_2, or by exporting from an un-contributed zkey if that produces equality) would prevent regressions.
| if (curve.G2.eq(zkey.vk_gamma_2, zkey.vk_delta_2)) { | ||
| throw new Error("SOUNDNESS ERROR: vk_gamma_2 and vk_delta_2 are equal. This zkey is insecure, it likely has no phase 2 contribution. Proofs can be forged. Run 'snarkjs zkey contribute' before exporting."); | ||
| } |
There was a problem hiding this comment.
Throwing here will skip the fd.close() in zkeyExportVerificationKey (it’s only reached on the success path), which can leak the underlying file handle when exporting a Groth16 VK from an insecure zkey. Consider wrapping the readBinFile/protocol switch in a try/finally so the descriptor is always closed, even when groth16Vk throws.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Following recent exploits in which users didn't contribute in phase 2 before extracting their verification key, I suggest adding a check and printing an error message in such cases.
Context:
https://x.com/PashovAuditGrp/status/2025598503255167195?s=20
https://x.com/blockaid_/status/2026937160079970737
https://blog.zksecurity.xyz/posts/groth16-setup-exploit/