Skip to content

fix: patch the query collision bug in multipoint opening#400

Merged
guorong009 merged 4 commits intomainfrom
gr@patch-multiopen-bug
Jul 7, 2025
Merged

fix: patch the query collision bug in multipoint opening#400
guorong009 merged 4 commits intomainfrom
gr@patch-multiopen-bug

Conversation

@guorong009
Copy link
Copy Markdown

@guorong009 guorong009 commented Jul 1, 2025

Description

The multipoint opening argument is used to efficiently evaluate multiple polynomial commitments at
multiple points. In the implementation, a map is used to track queries: (Commitment, QueryPoint) =>
Value
. Here, the key is a tuple of the polynomial commitment and the query point, and the value is the
evaluation of the polynomial at that point. A “query collision” occurs when two independent queries
have the same key—meaning they refer to the same commitment and the same evaluation point. When
this happens, one evaluation silently overwrites the other in the map, causing one to be ignored during
verification. This is a critical flaw: a malicious prover can exploit this to forge a proof, since one of the
evaluations will not be checked in the polynomial commitment opening.

Changes

  • catch the mismatching query evaluations in construct_intermediate_sets func in both shplonk and gwc schemes
  • add the tests exercising the bug

NOTE

The patch was co-authored by Jack (str4d) Grigg and Sean Bowe, and reviewed by Daira-Emma and Edu. The bug was reported by Suneal from zkSecurity. The patch is ported from here

@guorong009 guorong009 requested a review from ed255 July 1, 2025 06:07
@guorong009
Copy link
Copy Markdown
Author

@mratsim can you also review the PR? Thanks in advance!

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 92.14286% with 11 lines in your changes missing coverage. Please review.

Project coverage is 82.89%. Comparing base (495cbcd) to head (acb91eb).

Files with missing lines Patch % Lines
halo2_backend/src/poly/kzg/multiopen/gwc/prover.rs 33.33% 4 Missing ⚠️
...2_backend/src/poly/kzg/multiopen/shplonk/prover.rs 33.33% 4 Missing ⚠️
halo2_backend/src/poly/kzg/multiopen/shplonk.rs 87.50% 2 Missing ⚠️
halo2_backend/src/poly/multiopen_test.rs 98.80% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   82.82%   82.89%   +0.06%     
==========================================
  Files          76       76              
  Lines       16988    17110     +122     
==========================================
+ Hits        14071    14183     +112     
- Misses       2917     2927      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guorong009
Copy link
Copy Markdown
Author

@CPerezz @davidnevadoc
Can you also take a look at the PR if you have some time?

Copy link
Copy Markdown

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM! Great job porting these patches to our fork :)

@guorong009 guorong009 merged commit 198e9ae into main Jul 7, 2025
17 checks passed
@guorong009 guorong009 deleted the gr@patch-multiopen-bug branch July 7, 2025 11:29
@mitsu0925c74
Copy link
Copy Markdown

Great job @guorong009 💪

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.

4 participants