Upgrade scripts leave DEFAULT_ADMIN on the broadcast key

script/UpgradeSettlement.s.sol#L92-L120 script/UpgradeSpecialMarket.s.sol#L74-L107

Both upgrade scripts derive the new market's initial DEFAULT_ADMIN_ROLE holder from DEPLOYER_PRIVATE_KEY. They neither read ADMIN_ADDRESS nor call beginDefaultAdminTransfer() after deployment. ADMIN_TRANSFER_DELAY only configures the delay for future transfers; it does not initiate a handover.

Vulnerable Scenario:

1. An upgrade is executed using a broadcast account that is intended to relinquish control to a separate cold administrator.
2. The new market is registered and made operational, but the broadcast account remains its default admin and pendingDefaultAdmin() remains empty.
3. If that account is later compromised, the attacker can exercise the new market's administrative functions indefinitely.

Proof of concept: Save the following as test/poc/UpgradeAdminRetention.t.sol and run forge test --match-path test/poc/UpgradeAdminRetention.t.sol -vv:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.30;

import {Test, Vm} from "forge-std/Test.sol";
import {UpgradeSettlement} from "../../script/UpgradeSettlement.s.sol";
import {PlakxioBadge} from "../../src/PlakxioBadge.sol";
import {PlakxioSettlement} from "../../src/PlakxioSettlement.sol";
import {MockUSDC} from "../mocks/MockUSDC.sol";

contract UpgradeAdminRetention is Test {
    event SettlementContractSet(address indexed settlement, bool allowed, uint256 classMask);

    function test_upgradeIgnoresTheConfiguredColdAdmin() public {
        uint256 deployerKey = 0xD00D;
        address deployer = vm.addr(deployerKey);
        address coldAdmin = makeAddr("coldAdmin");
        MockUSDC usdc = new MockUSDC();

        vm.prank(deployer);
        PlakxioBadge badge = new PlakxioBadge(deployer, "uri", 2 days, 1 days);

        vm.setEnv("BADGE_ADDRESS", vm.toString(address(badge)));
        vm.setEnv("USDC_ADDRESS", vm.toString(address(usdc)));
        vm.setEnv("TREASURY_ADDRESS", vm.toString(makeAddr("treasury")));
        vm.setEnv("RELAYER_ADDRESS", vm.toString(makeAddr("relayer")));
        vm.setEnv("DEPLOYER_PRIVATE_KEY", vm.toString(deployerKey));
        vm.setEnv("ADMIN_ADDRESS", vm.toString(coldAdmin));

        vm.recordLogs();
        new UpgradeSettlement().run();
        Vm.Log[] memory logs = vm.getRecordedLogs();
        address settlement;
        for (uint256 i = 0; i < logs.length; ++i) {
            if (
                logs[i].emitter == address(badge) && logs[i].topics.length == 2
                    && logs[i].topics[0] == SettlementContractSet.selector
            ) settlement = address(uint160(uint256(logs[i].topics[1])));
        }

        PlakxioSettlement market = PlakxioSettlement(settlement);
        assertEq(market.defaultAdmin(), deployer);
        (address pending,) = market.pendingDefaultAdmin();
        assertEq(pending, address(0));

        address redirectedTreasury = makeAddr("redirectedTreasury");
        vm.prank(deployer);
        market.setTreasury(redirectedTreasury);
        assertEq(market.treasury(), redirectedTreasury);
    }
}

Observed on the frozen source:

[PASS] test_upgradeIgnoresTheConfiguredColdAdmin()
Suite result: ok. 1 passed; 0 failed; 0 skipped

The same retained-admin state was reproduced through UpgradeSpecialMarket.run().

A later compromise of the broadcast key can redirect future treasury commissions and change fee, floor, or escalation parameters on the replacement market. The key cannot seize escrowed principal because the markets retain no principal between calls. Recovery requires the legitimate badge administrator to deauthorize and replace the affected market.

Read and validate ADMIN_ADDRESS before starting the broadcast, then begin the two-step handover on each new market before stopBroadcast():

 address deployer = vm.addr(deployerKey);
+address finalAdmin = vm.envAddress("ADMIN_ADDRESS");
+require(finalAdmin != address(0) && finalAdmin != deployer, "ADMIN_ADDRESS must be a separate cold admin");
 vm.startBroadcast(deployerKey);
 // deploy and configure the new market
+market.beginDefaultAdminTransfer(finalAdmin);
 vm.stopBroadcast();

Apply the same sequence to both upgrade scripts and log that the cold administrator must accept after the configured delay. Update the settlement upgrade comment so it does not claim parity with the base deployment until the handover is actually initiated. Add tests for both upgrade paths, zero and deployer-equal addresses, and delayed acceptance.

This makes upgrades explicitly two-phase and requires a second cold-admin transaction after the delay. Because the change is confined to deployment scripts, it does not increase the deployed market bytecode.

Fixed. The upgrade scripts call _readHandover(deployer, true), so an unset ADMIN_ADDRESS reverts AdminAddressUnset instead of defaulting, and beginDefaultAdminTransfer runs on every target. The shared AdminHandover module also refuses a handover in flight to a different admin and an admin that cannot accept.

Final disposition: Fixed. Verified at audit-v3-rereview-handoff, commit f020448976eb7c4895e1626ef3228876cff2b7d2. This earlier remediation disposition is carried forward from the prior re-review, not independently re-verified in the generation-d pass. The report-wide scope and evidence limitations remain applicable; see the final report overview.