Root + Impact
The entire CORRUPTED sweep — including stakers' principal — can be redirected to an address the sponsor controls, after stakers have already committed funds and are locked in. Stakers who verified recoveryAddress at deposit time have no guarantee it remains the sweep destination.
Description
-
The pool sponsor sets recoveryAddress at pool creation. This address receives all CORRUPTED-path sweeps, excess bonus, and post-resolution donations. Every other owner-mutable parameter (expiry, scope) has a one-way lock latch that engages once the pool is live.
-
recoveryAddress has no lock analogous to expiryLocked or scopeLocked. The owner can change it at any time — after stakers have deposited, after resolution, and even between a CORRUPTED flag and the subsequent sweep. Under bad-faith CORRUPTED, the sponsor can front-run claimCorrupted() with a setRecoveryAddress call to redirect the entire pool (stakers' principal + bonus) to an arbitrary address.
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
@>
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
recoveryAddress = newRecoveryAddress;
}
Risk
Likelihood:
Reason 1 — The pool sponsor is a single address with full control over `recoveryAddress` for the pool's entire lifetime. No multisig or timelock is required by the contract.
Reason 2 — `recoveryAddress` is the destination for the entire pool balance under bad-faith CORRUPTED. The sponsor can front-run `claimCorrupted()` to redirect the sweep to an address they control.
Impact:
Impact 1 — Under bad-faith CORRUPTED, the sponsor can redirect the full pool (stakers' principal + bonus) to an arbitrary address after stakers are locked in. Stakers who verified `recoveryAddress` at deposit time have no assurance it remains the sweep destination.
Impact 2 — The asymmetry with `expiryLocked`/`scopeLocked` is inconsistent: stakers can trust expiry and scope are immutable post-deposit, but `recoveryAddress` — which controls the CORRUPTED fund destination — is perpetually mutable.
Proof of Concept
function test_L2_SetRecoveryAddress_NoLock() public {
_stake(alice, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
address originalRecovery = pool.recoveryAddress();
address newRecovery = makeAddr("newRecovery");
pool.setRecoveryAddress(newRecovery);
uint256 newRecoveryBefore = token.balanceOf(newRecovery);
pool.claimCorrupted();
assertGt(token.balanceOf(newRecovery) - newRecoveryBefore, 0);
assertEq(token.balanceOf(originalRecovery), 0);
}
Forge output:
[PASS] test_L2_SetRecoveryAddress_NoLock()
Owner redirected CORRUPTED sweep after resolution
Recommended Mitigation
Add a one-way recoveryAddressLocked latch that engages on the first stake, mirroring the existing expiryLocked pattern. Once locked, the CORRUPTED sweep destination is immutable for the pool's remaining lifetime.
+ bool public recoveryAddressLocked;
function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (!expiryLocked) {
expiryLocked = true;
+ recoveryAddressLocked = true;
}
}
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ if (recoveryAddressLocked) revert RecoveryAddressLocked();
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
recoveryAddress = newRecoveryAddress;
}
