Skip to content

fix: Verify prover-supplied FRI query points#17

Open
Tabaie wants to merge 3 commits into
mainfrom
fix/fri/query-soundness
Open

fix: Verify prover-supplied FRI query points#17
Tabaie wants to merge 3 commits into
mainfrom
fix/fri/query-soundness

Conversation

@Tabaie

@Tabaie Tabaie commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Currently the verifier doesn't double-check that the FRI query leaf IDs in the proof are correctly computed from the transcript. This PR adds that check and a regression test that would have caught the soundness issue. It also consolidates some planning logic duplicated in the Base and Ext cases into the same function.

I would have preferred to change the Merkle proof struct and the Verify function slightly so that the leaf index is not part of the proof at all, as it is conceptually part of the statement. So ideally the signature would be Verify(root hash.Digest, leaf hash.Digest, leafIdx int, proof Proof, nh NodeHasher) bool. But trying this created integration issues because it seems like some components outside FRI use the same query indices.


Note

High Risk
This changes cryptographic FRI verification behavior and closes a soundness issue; incorrect checks could break valid proofs or leave a vulnerability open.

Overview
Closes a FRI soundness gap where the verifier accepted Merkle openings without confirming that Path.LeafIdx matches the query position derived from the Fiat–Shamir transcript (sbase = s % (N_j/2)). checkQuery / checkQueryExt now reject running-path and extra-level openings when the prover-supplied leaf index disagrees with that value; extra levels use checkLevelQueryBase on the shared levelIntroductions plan.

Prove/Verify planning is refactored so newLevelIntroductions builds round↔level mappings once (replacing duplicated logic in buildProvePlan and Verify). A regression test TestCheckQueryRejectsOpeningAtWrongLeafIndex documents the prior bug.

Reviewed by Cursor Bugbot for commit 7ce8b21. Bugbot is set up for automated code reviews on this repo. Configure here.

@Tabaie Tabaie requested a review from ThomasPiellard June 5, 2026 14:30
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.

1 participant