Zk Verifier Bugs

---
name: zk-verifier-bugs
description: Detect ZK proof-verifier contract bugs — missing public-input binding (unconstrained input → forgery), proof malleability, BN254 field-element range checks (input >= field modulus), unchecked pairing/ecAdd/ecMul precompile returns, nullifier reuse / double-spend, verification-key upgradeability, and trusted-setup assumptions. Activate whenever a contract verifies a SNARK/STARK proof, calls the bn256/altbn128 precompiles, or consumes nullifiers.
---

# ZK verifier contract bug detection

## When this applies

Trigger on any of:

- On-chain Groth16 / PLONK / Halo2 verifiers (`verifyProof`, `verify`)
- Calls to precompiles `0x06` (ecAdd), `0x07` (ecMul), `0x08` (pairing) over BN254/alt_bn128
- Public-input arrays passed alongside a proof
- Nullifier / commitment sets (mixers, privacy pools, rollup exits)
- Upgradeable verification keys (vk) or governance-set verifiers
- Bridges/rollups that settle based on a validity proof

## Detection patterns

### Unconstrained / unbound public input (CRITICAL)
```solidity
function withdraw(uint256[8] proof, uint256 root, uint256 nullifier, address recipient) external {
    require(verifier.verifyProof(proof, [root, nullifier]));  // ← recipient NOT in inputs
    payable(recipient).transfer(amount);
}
```
**Signal:** a value the contract acts on (`recipient`, `amount`, `chainId`) is not among the proof's public inputs. The proof says nothing about it, so an attacker reuses a valid proof with a different recipient — front-runnable theft. Every value the proof is supposed to authorize must be a bound public input.

### Missing BN254 field-element range check (HIGH)
```solidity
// public input passed straight to the verifier without bounds
require(input < SNARK_SCALAR_FIELD);   // ← if MISSING:
```
**Signal:** public inputs (and proof point coordinates) must be `< r` (the BN254 scalar/base field modulus, `21888242871839275222246405745257275088548364400416034343698204186575808495617`). An out-of-range input wraps mod r, letting a different field element pass as a valid input — input forgery. Verifiers must `require(x < FIELD_MODULUS)` for every input.

### Unchecked precompile return value (HIGH)
```solidity
assembly { let ok := staticcall(gas(), 0x08, ...) }  // 'ok' / returndata ignored
```
**Signal:** the pairing precompile (`0x08`) returns success flag + a 32-byte result that must equal 1; `ecAdd`/`ecMul` return whether the input points were valid. Ignoring the success bool or the pairing result accepts invalid proofs (precompile reverts on malformed points only sometimes). Always check `success` AND the returned value.

### Nullifier reuse / double-spend (CRITICAL)
```solidity
require(verifier.verifyProof(...));
// ← no check that nullifier was unused; no marking after
payout(recipient);
```
**Signal:** nullifier not checked against `spent[nullifier]` before payout, or not marked spent after (or marked after an external call → reentrant double-spend). Same note must not redeem twice.

### Proof malleability (MEDIUM)
Groth16 proofs are malleable — a valid `(A,B,C)` can be transformed to another valid proof for the same inputs. If proof bytes are used as a dedup/replay key, the twin bypasses it. Dedup on public inputs / nullifiers, never on proof bytes.

### Mutable vk / verifier (HIGH)
An upgradeable or owner-settable verification key lets an admin swap in a vk that validates forged proofs — a centralization backdoor over the entire system's soundness.

## Severity rubric

| Pattern | Severity | Notes |
|---|---|---|
| Action value not a bound public input | **Critical** | Proof reuse → theft |
| Nullifier not checked / marked late | **Critical** | Double-spend |
| Missing BN254 field range check | **High** | Input forgery via wraparound |
| Unchecked pairing/ecMul return | **High** | Invalid proofs accepted |
| Mutable vk without timelock/governance | **High** | Soundness backdoor |
| Proof bytes used as replay key (malleable) | **Medium** | Twin-proof bypass |

## Remediation patterns

1. **Bind every authorized value** as a public input committed in-circuit (`recipient`, `amount`, `chainId`, `contractAddress`); verify the input vector matches what the contract acts on.
2. **Range-check all public inputs** `< FIELD_MODULUS` (BN254 `r`) before verification — use the snarkjs/circomlib generated verifier's checks; don't strip them.
3. **Check precompile results** — `require(success)` and `require(out == 1)` for pairing; validate `ecAdd`/`ecMul` success.
4. **Nullifier set:** `require(!spent[n]); spent[n] = true;` *before* any external call (CEI).
5. **Dedup on inputs/nullifiers, not proof bytes**, to neutralize Groth16 malleability.
6. **Govern the vk** behind a timelock/multisig; document the trusted-setup ceremony and its toxic-waste assumptions.

## False-positive notes

- A verifier generated by snarkjs/Circom that already includes the `< r` range checks and full pairing-result check is sound for those concerns — don't re-flag the precompile calls.
- STARK/transparent systems have no trusted-setup assumption — don't flag ceremony concerns there.
- An immutable, audited vk constant is fine — flag only mutable/owner-settable vks.

## Related

- [[signature-malleability]] — proof malleability mirrors ECDSA malleability for replay keys
- [[cross-chain-messaging]] — rollup/bridge settlement consumes these proofs
- [[centralization-risk]] — mutable verification keys are an admin soundness backdoor