Severity: Medium
Component: ghost-dao-contracts @ b87dff4c7a6ac34b35698c42b4d1f34b5f930864 (GhostAuthority.sol lines 25 to 43, FatsoERC20.sol lines 22 to 29)
Source review only. No transactions. I verified the behavior with a local Foundry test against the pinned sources, 5 of 5 runs identical.
Description
All four role setters, pushGovernor, pushGuardian, pushPolicy and pushVault, write the new address in the same call. There is no pending slot and no pull or accept function anywhere in the contract, although the event names GovernorPushed, GuardianPushed, PolicyPushed and VaultPushed follow the two step convention those names usually imply. A mistyped or squatted address is live immediately and cannot be recovered, because the previous holder has already lost the role. The vault is the onlyVault minter of FTSO with no amount cap, so a bad vault write makes the wrong address the unlimited minter.
function pushGovernor(address newGovernor) external onlyGovernor {
emit GovernorPushed(governor, newGovernor);
governor = newGovernor;
}
function pushVault(address newVault) external onlyGovernor {
emit VaultPushed(vault, newVault);
vault = newVault;
}
Proof of concept (a Foundry test deploying the pinned GhostAuthority and calling pushGovernor as the governor and as an outsider; local chain only. 5 of 5 runs byte identical):
// poc/test/TrackB.t.sol
function test_pushGovernorImmediate_nonGovernorReverts() public {
GhostAuthority auth = new GhostAuthority(initialGovernor, guardian, policy, vault);
address newGov = address(0x1111);
vm.prank(initialGovernor);
auth.pushGovernor(newGov);
assertEq(auth.governor(), newGov); // effective in the SAME transaction, no acceptance step
vm.prank(attacker);
vm.expectRevert(GhostAuthority.Unauthorized.selector);
auth.pushGovernor(address(0x2222)); // only the (already replaced) governor can push
assertEq(auth.governor(), newGov);
}
Raw output of the 5 runs (identical):
[PASS] test_pushGovernorImmediate_nonGovernorReverts() (gas: 785948)
Logs:
ASSERT pushGovernor set governor==newAddr immediately; non-governor Unauthorized
governor 0x0000000000000000000000000000000000001111
The governor variable reads 0x…1111 in the same transaction as the pushGovernor call, proving there is no pending or acceptance step anywhere in the flow, and a non governor call reverts Unauthorized leaving the variable unchanged.
Steps to reproduce
- Clone and pin:
git clone https://git.ghostchain.io/ghostchain/ghost-dao-contracts.git && git -C ghost-dao-contracts checkout b87dff4c7a6ac34b35698c42b4d1f34b5f930864
- Create the Foundry test above against the repo’s
GhostAuthority(remappings atdependencies/). - Run:
forge test --match-test test_pushGovernorImmediate_nonGovernorReverts -vv
- Observe
auth.governor()equals the new address in the same transaction as thepushGovernorcall (no pending slot, no acceptance step), and the non governor call revertsUnauthorized. - Run 5 times; byte identical output.
Impact
Toy numbers: the governor fat fingers pushVault to a squatted address. That address can immediately call Fatso.mint(anyone, 10_000_000e9) and mint unlimited FTSO, diluting and unbacking the supply, and the governor cannot correct the mistake unless they still hold the governor role. The same one step shape applies to governor, guardian and policy, where a lost role is unrecoverable without redeployment.
Fix
Implement a two step handover for all four roles, a pending slot written by push and an acceptRole() call by the new address, as the event vocabulary already implies.