Root + Impact
Description
-
When a sponsor creates a pool through ConfidencePoolFactory.createPool, the factory checks that the caller currently owns the underlying Safe Harbor agreement, then makes that same address the pool's owner for its entire lifetime. As pool owner, that address controls recoveryAddress (the destination for a bad-faith CORRUPTED sweep), expiry until the first stake, pool scope until it locks, and pause/unpause.
The problem is that ownership check only runs once, at creation time. The agreement contract is Ownable2Step and its own documentation says it is meant to support handing management to a different address than the original deployer, for example a protocol sale or a DAO moving control to a new team. Once that transfer happens on the agreement side, the pool has no way to find out. The original owner keeps full, permanent control of the pool even after they have no relationship left to the protocol it backs
function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
@> if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt);
IConfidencePool(pool)
.initialize(
agreement,
stakeToken,
address(safeHarborRegistry),
defaultOutcomeModerator,
expiry,
minStake,
recoveryAddress,
@> msg.sender,
accounts
);
Inside ConfidencePool.initialize, that address gets locked in as the permanent owner with no link back to the agreement:
recoveryAddress = recoveryAddress_;
outcome = PoolStates.Outcome.UNRESOLVED;
_replaceScope(accounts);
@> _transferOwnership(owner_);
And setRecoveryAddress never checks against the agreement, only against the pool's own cached owner:
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
address oldRecoveryAddress = recoveryAddress;
@> recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
Risk
Likelihood:
-
This triggers whenever an agreement's ownership changes after a pool already exists for it. Agreement.sol is Ownable2Step specifically so management can be handed to a different address than the deployer, so this is a normal business event for the dependency, not a rare edge case.
-
The old owner does not need to time anything or watch the pool. They can call setRecoveryAddress once, right after losing agreement ownership, then walk away completely. The theft fires later, automatically, whenever the pool happens to resolve bad-faith CORRUPTED.
Impact:
-
The full pool balance, every staker's principal plus the entire bonus pot, gets swept to an address controlled by someone who no longer has any relationship to the protocol.
-
The agreement's new, current owner has zero on-chain recourse. There is no function anywhere in the factory or the pool that lets them reclaim pool ownership or override the stale owner's changes.
-
The same stale ownership carries over to the rest of the owner-only surface. The old owner can still pause the pool to block new stakers, for free, with nothing to gain except sabotage.
Proof of Concept
function testStaleAgreementOwnerHijacksRecoveryAfterLegitimateOwnershipTransfer() external {
uint256 expiry = block.timestamp + 31 days;
vm.prank(alice);
address pool = factory.createPool(address(agreementContract), address(token), expiry, ONE, recovery, _scope());
assertEq(ConfidencePool(pool).owner(), alice);
agreementContract.setAgreementOwner(bob);
assertEq(agreementContract.owner(), bob);
assertEq(ConfidencePool(pool).owner(), alice);
token.mint(victim, 100 * ONE);
vm.startPrank(victim);
token.approve(pool, 100 * ONE);
ConfidencePool(pool).stake(100 * ONE);
vm.stopPrank();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
ConfidencePool(pool).pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
ConfidencePool(pool).flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
vm.prank(alice);
ConfidencePool(pool).setRecoveryAddress(aliceWallet);
ConfidencePool(pool).claimCorrupted();
assertEq(token.balanceOf(aliceWallet), 100 * ONE);
assertEq(token.balanceOf(pool), 0);
assertEq(token.balanceOf(recovery), 0);
}
Recommended Mitigation
New error, added alongside the existing ones in IConfidencePool.sol:
error InvalidRecoveryAddress();
+ error NotAgreementOwner();
error PoolNotExpired();
New function, added right after setRecoveryAddress in ConfidencePool.sol:
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
+
+ /// @notice Lets the agreement's current, live owner reclaim pool ownership if it has
+ /// drifted from a stale creation time snapshot.
+ function syncOwnerToAgreement() external {
+ address currentAgreementOwner = IAgreement(agreement).owner();
+ if (msg.sender != currentAgreementOwner) revert NotAgreementOwner();
+ _transferOwnership(currentAgreementOwner);
+ }
IAgreement needs importing into ConfidencePool.sol for this, it is currently only imported in the factory.
