[Audit] [MEDIUM] deploy-all verifier can pass while mandatory components and ownership checks are skipped

[mejango]

Audit seed

Deep Nemesis / deploy-all-v6 / all script/**/*.sol and src/**/*.sol / deployment verifier and wiring

Repos involved

• deploy-all-v6

Root cause

script/Verify.s.sol treats some canonical deployment outputs and ownership checks as optional verifier inputs:

• VERIFY_SAFE, VERIFY_ADDRESS_REGISTRY, and VERIFY_DEFIFA_DEPLOYER use vm.envOr(..., address(0)).
• The production-chain fail-closed env block does not require those values.
• _verifyAddressRegistryAndDefifa() converts zero values into _skip() instead of failure.
• _printSummary() reports ALL CHECKS PASSED whenever _failed == 0; skipped checks do not prevent success.

This diverges from Deploy.s.sol, which deploys the address registry, wires it into downstream deployers, deploys Defifa infrastructure on supported chains, and uses safeAddress() as the authority anchor for multiple singleton owners.

Impact

A post-deploy verification run can produce a clean success signal without proving:

• the canonical JBAddressRegistry exists and is the one used by provenance-dependent deployers,
• the Defifa deployer exists and owns/controls the expected game initialization surface,
• project and singleton ownership converged to the intended safe.

This is not a direct runtime fund-drain primitive, but it is a deployment-assurance failure in the deployment-orchestrator repo. Operators can certify an incomplete or authority-drifted topology as valid.

Proof of concept

1. Run deployment or resume.
2. Run Verify.s.sol with the required core env vars but omit VERIFY_ADDRESS_REGISTRY, VERIFY_DEFIFA_DEPLOYER, and/or VERIFY_SAFE.
3. The verifier skips those categories.
4. If the other checks pass, _printSummary() emits Result: ALL CHECKS PASSED.

Relevant code:

• script/Verify.s.sol:280-286: optional env loading
• script/Verify.s.sol:288-306: production required-env block excludes these values
• script/Verify.s.sol:675-689: skipped address registry and Defifa checks
• script/Verify.s.sol:908-915: safe ownership check only runs if expectedSafe != address(0)
• script/Verify.s.sol:946-969: skipped checks do not block ALL CHECKS PASSED

Why this survived self-review

The strongest counterargument is that these env values may be intentionally optional for testnets. That does not hold for the reported condition because production-chain required-env checks already exist, but they omit these mandatory deployment outputs. Also, the deployment script itself does not make the address registry optional.

Recommended fix

Require canonical deployment outputs on supported/production chains, and make skipped mandatory checks impossible:

expectedSafe = vm.envAddress("VERIFY_SAFE");
addressRegistry = vm.envAddress("VERIFY_ADDRESS_REGISTRY");
defifaDeployer = vm.envAddress("VERIFY_DEFIFA_DEPLOYER");

_check(addressRegistry.code.length > 0, "AddressRegistry has code", true);
_check(defifaDeployer.code.length > 0, "DefifaDeployer has code", true);

Add explicit owner() checks for safe-owned singleton contracts such as JBDirectory, JBPrices, JBFeelessAddresses, JBBuybackHookRegistry, JBSuckerRegistry, and router registries where deployed.