[mjthatch]

Security audit run 2 (Cloudflare security-audit skill, standard profile, on e562c3b) found no soundness or binding gap in the generated Groth16 and fflonk programs. It confirmed four findings in the off-chain tools and left one lead needing validation. Each record was reproduced by a fresh verifier and checked again by a second one. This PR fixes all five. The full reports are in build/security-audit-2/.

Generated programs and witnesses are unchanged: every program and witness the tools write from the committed examples hashes the same before and after.

Noir: tools/noir_r1cs.py

The converted artifact is the checked one (medium). The opcode check read the ACIR that nargo compile --print-acir printed. The tool then converted target/<name>.json, with the name taken by a regex over Nargo.toml. A name = "..." line inside a multi-line description, plus a planted target/ file, got a trivial circuit keyed and turned into a program, while the tool reported the reviewed circuit's opcodes. nargo now compiles into a fresh temporary --target-dir, and the tool converts and keeps the one artifact that run wrote. It no longer reads the circuit's target/ or parses Nargo.toml.

Public signals are the public parameters (medium). noir-cli makes the first ACIR witnesses the R1CS public signals, whatever their visibility. So a circuit following the old rule ("the sighash first among the public inputs") could still publish a private parameter in the spend: for example fn main(sighash: pub Field, a: Field, c: pub Field, b: Field) gave public signals [sighash, a]. A return value had the same effect. compile now refuses unless func 0's public parameters are w0..w(k-1) and nothing is returned. It also checks that the R1CS header has nPubOut 0 and nPubIn k. The rule in docs/noir.md is now: public parameters first, the sighash first of all, no return value. The docstring's stale two-limb / --sighash-inputs text is corrected.

nargo's warnings are fatal (low). nargo 1.0.0-beta.19 reports a Brillig output left unconstrained as bug: on stderr and exits 0, and the tool discarded stderr on success. compile now passes --deny-warnings and also refuses on any bug/error/warning line. nargo's check is a heuristic: it flags some constrained hints, such as one checked under an if. For a reviewed circuit, --allow-nargo-warnings prints the warnings instead of refusing. With --deny-warnings, an unsafe block needs nargo's // Safety: comment. The toy circuit has none and is unaffected: its R1CS is byte-identical to examples/noir/multiplier.r1cs.

noir-cli by its full commit (needs validation). The install pinned --rev 4b7caac. cargo resolves a short rev as a name, and libgit2 1.9.2 lets a tag of that name take precedence. The docs and the tool now give 4b7caace1f2128e454c8d0fe50cac1ec46b1e272.

fflonk: groth/core/snarkjs.py, tools/fflonk_snarkjs.py

Evaluations are read as snarkjs reads them (low). fflonk_proof promised evaluations "reduced mod r" but only applied int(). With --pad N, which skips the hand run, the tool printed "the proof verifies" for a + r, a - r and a + 6r, and wrote witness words the program refuses or the harness cannot load. Evaluations are now decimal strings below 2^256, reduced mod r. fflonk_proof checks curve as the Groth16 loader does. dsl.wv refuses a value outside [0, 2^bits). An out-of-range --pad no longer truncates an earlier witness file before it is refused.

Affine points only (hardening from the same audit). snarkjs writes every point with z = 1, and reads any other z as Jacobian (X/Z², Y/Z³). The loaders divided once, (X/Z, Y/Z). A key edited with z ≠ 1 could verify in snarkjs yet compile to a different IC0, with the same program-time report. g1/g2 now refuse z ≠ 1.

Tests

• tests/test_noir.py:
  • Layout check: the three bad layouts are refused and the multiplier passes.
  • nargo run, stubbed: the artifact is the run's own and not a decoy under target/; a bug: on stderr with exit 0 is refused, as are zero or two built programs; --allow-nargo-warnings prints instead of refusing.
  • R1CS header: the header of examples/noir/multiplier.r1cs is read correctly.
• tests/test_tools.py:
  • a + r and a + 4r load as a;
  • a - r, a + 6r, hex, ' 1', '1_2', JSON numbers and booleans are refused, and so is a proof labelled bls12381;
  • wv(256, -1) and wv(256, 2**256) are refused;
  • z ≠ 1 is refused for G1 and G2;
  • --pad 512 leaves an earlier witness file as it was.
• Checked by hand with the real nargo 1.0.0-beta.19 and noir-cli:
  • the toy circuit compiles to the committed R1CS;
  • the crafted directory now gets the honest R1CS;
  • the (sighash, a, c, b) layout is refused;
  • an under-constrained hint is refused, and accepted with --allow-nargo-warnings.
• ruff, mypy and the full test suite (all 16 layers, with hand) pass.

Not in this PR

The audit's hardening notes are listed in build/security-audit-2/REPORT.md §6. They include witness malleability (re-padding an fflonk spend cuts its feerate), the unchecked fflonk key constants, degenerate-setup refusals, program-time spendability estimates, and the circom toolchain identity.

🤖 Generated with Claude Code