/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)

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)

// 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)

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)

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