audit(foundry): elevate H-1/H-2 to Critical, add real gas measurements

[Abidoyesimze]

Summary

Follow-up to the review on #22. Two changes, both to Documentation/technical/audits/foundry-src-security-review.md plus the PoC test that backs the second one.

1. Severity: agreed, H-1 and H-2 → Critical

You're right, and I'd land on the same call independently: both permanently brick a Global Iteration on contracts that can't be upgraded, with zero on-chain recovery path, and both are cheap (H-1: ~0.00001 ETH per Sybil identity) or free (H-2: any assigned aggregator submitting a routine, possibly-non-malicious zero CID) for a single unprivileged participant to trigger. "Permanent brick + trivial-to-cheap trigger + no recovery path" is a Critical bar, not a High one — the DoS-vs-fund-loss distinction some frameworks use doesn't hold up well here given task contracts have no upgrade path at all.

Renumbered: H-1 → C-1 (Sybil-cheap gas-DoS), H-2 → C-2 (zero-CID collision), H-3 → H-1 (missing quorum), H-4 → H-2 (predictable randomness). Summary table and every cross-reference updated. No finding content changed as part of the renumbering, only labels.

2. Gas number: real forge measurements, not just the closed-form estimate

Added test_gas_finalizeEvaluation_and_slashAuditors_atScale (foundry/test/SecurityFindings.t.sol). One thing worth flagging explicitly: Params.auditorsPerBatch/modelsPerBatch is hardcoded to the demo defaults (3, 3) in the DINTaskAuditor constructor with no setter, so a batch with literal spec-scale internal dimensions (auditorsPerBatch=10, modelsPerBatch=100) can't actually be deployed without modifying the contract — out of scope for a findings-only pass. So this couldn't be a single clean measurement; it's two steps, kept explicitly separate in the report:

1. Measured: real gas at two registrant scales (10 batches / 30 auditors, and 30 batches / 90 auditors), holding batch-internal-size at demo's 3×3. Gives a real marginal cost of 40,240 gas/batch (finalizeEvaluation) and 30,509 gas/batch (slashAuditors), and a real per-(auditor,model)-pair cost of 4,471 gas.
2. Extrapolated (structural, Big-O, explicitly labeled as such — not a second measurement): scale that real per-pair cost by the spec-scale pair count (1,000 vs. demo's 9) and batch count (50 vs. 10/30 measured) to project full spec scale.

Result: ≈223.5M gas (finalizeEvaluation) and ≈169.5M gas (slashAuditors, and that's a floor — its early-break-on-first-miss behavior under an all-Sybil-never-votes attack undermeasures the function's true worst case, which pushes toward the finalizeEvaluation figure instead). Both are 5.6×–7.5× over Optimism's ~30M block limit. This corroborates and quantifies the original estimate with real data rather than changing its conclusion.

3. Your UpgradeValidation.t.sol FFI/npx note

Reproduced independently in a second environment — same failure class (OZ upgrades-core's readBuildInfo/checkOutputSelection via FFI), confirmed unrelated to any contract or PoC in this report. SecurityFindingsTest has no such dependency.

Verification

forge test --match-contract SecurityFindingsTest -vv

10/10 passing (9 from #22 + the new gas-measurement test).

Scope

Report + PoC test only, same findings-only boundary as #22. No contract in foundry/src/ touched.

[umeradl]

Reviewed and merged into develop — thanks, Similoluwa.

Ran forge test --match-contract SecurityFindingsTest -vv locally: 10/10 passing, including the new test_gas_finalizeEvaluation_and_slashAuditors_atScale, and the projected figures match the report exactly (≈223.5M gas for finalizeEvaluation, ≈169.5M for slashAuditors at full spec scale — both well past Optimism's ~30M block limit).

Agreed on both calls:

• C-1/C-2 severity bump is right — permanent brick, no upgrade path, cheap-to-free trigger is a Critical bar.
• Keeping the measured-vs-extrapolated split explicit in the report (real numbers at 10/30 batch scale, structurally projected to spec scale) is the correct way to present this rather than dressing up an estimate as a measurement.

Since your branch was already based on top of #22's merge commit and touched no other files, this merged cleanly with no manual reconciliation needed.