Description

In common/utils/plonk_util.go, two functions CalculateAppVkHashForBn254 and CalculateAppVkHashForBn254InCircuit are used for hashing a replonk.VerifyingKey, the former natively, the latter in circuit. However, they are missing some fields of the verifying key: Size, SizeInv, Generator, Qcp, and CommitmentConstraintIndexes. Because these are not hashed, two verifying keys that differ only in those fields will hash to the same value.

Impact

An attacker, given a verifying key for a circuit and public inputs for it, may, without being able to find witnesses that satisfy the constraint system for that circuit, be able to change the above fields of the verifying key in such a way that they can produce a proof for this modified verifying key and the same public inputs. This incorrect verifying key would then still hash to the same value as the legitimate one, and thus possibly not be distinguished by the system.

In the code that was in scope for this audit, the verifying key hashes are used in communication with the prover (local server or Brevis partner flow). If these hashes are then ultimately also used to identify the proven circuit on chain, then this issue could allow attackers to get incorrect statements accepted on chain. As on-chain components were not part of this engagement, we have not checked whether this is the case.

Recommendations

We recommend to hash all fields of the verifying key. Optimally, gnark would offer a function to do this, as this would make it less likely that future gnark extensions that add fields to the verifying key reintroduce this issue.

Remediation

This issue has been acknowledged by Brevis, and a fix was implemented in commit 05e2b9f9↗.