The withdraw() function emits a Withdrawn event, but there's no check or event indicating WHICH treasures were claimed when withdrawing remaining funds. More critically, if claimsCount >= MAX_TREASURES but some treasures weren't actually claimed (due to the bug #3), the owner could withdraw funds that should have been available for legitimate claimants.
Likelihood:
Reason 1
Reason 2
Impact:
Impact 1: While the comment says this is intentional, having an unused public input is confusing
Impact 2: The recipient binding relies on it being part of the public inputs that get verified, but there's no explicit constraint showing this relationship.
Add an explicit constraint or documentation clarifying how the recipient provides replay protection.
The claim that the proof system is broken because the recipient is not explicitly constrained in the Noir circuit reflects a misunderstanding of how zero-knowledge proofs bind public inputs. Although the circuit does not impose algebraic constraints on recipient, the value is still included in the public input vector, which is cryptographically committed to during proof generation. As a result, the proof is only valid for the exact tuple of public inputs it was created with. Any attempt by an attacker to front-run and substitute a different recipient would alter this tuple, causing the verifier’s check to fail because the proof no longer matches the provided public inputs. Therefore, while unconstrained public inputs do not enforce logical relationships within the circuit, they remain inseparably bound to the proof itself, and this binding is sufficient to prevent tampering or replay with modified values. Run the unit tests 'testClaimInvalidProofFails', 'testFrontRunningClaimFails'.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.