BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: medium

Recovery address remains mutable after withdrawals are disabled, allowing sponsor to redirect corrupted-resolution funds after stakers are locked

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: Stakers should be able to verify pool parameters before depositing. Once the registry reaches UNDER_ATTACK, withdrawals are permanently disabled and stakers are locked into the pool until final resolution.

  • Issue: setRecoveryAddress() remains callable even after withdrawals are disabled. This allows the pool owner to change the recoveryAddress fter stakers can no longer exit. If the pool later resolves as bad-faith CORRUPTED, claimCorrupted() sends the full pool balance to the newly configured address.

function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
address oldRecoveryAddress = recoveryAddress;
​
@> recoveryAddress = newRecoveryAddress;
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
​
if (
@> riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
@> revert WithdrawsDisabled();
}
​
...
}
function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
​
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();
​
@> stakeToken.safeTransfer(recoveryAddress, toSweep);
​
emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
}

Risk

Likelihood:

  • This occurs when the registry reaches UNDER_ATTACK, withdrawals become permanently disabled, and the pool owner changes recoveryAddress before corrupted settlement.

  • The function has no lifecycle restriction, so the owner can update the recovery destination at any time before claimCorrupted() transfers the pool balance.

Impact:

  • Stakers can no longer withdraw, but the destination of their principal and bonus can still be changed.

  • In a bad-faith CORRUPTED resolution, the full pool balance is swept to the new recovery address instead of the address stakers saw when they deposited.

Proof of Concept

Explanation:
The test shows that after risk starts and withdrawals are disabled, the owner can still change recoveryAddress. When the pool later resolves as CORRUPTED, claimCorrupted() sends all funds to the new address.

function testOwnerCanChangeRecoveryAddressAfterWithdrawalsAreDisabled() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 40 * ONE);
​
_passThroughUnderAttack();
​
address newRecovery = makeAddr("newRecovery");
​
pool.setRecoveryAddress(newRecovery);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
uint256 newRecoveryBefore = token.balanceOf(newRecovery);
​
pool.claimCorrupted();
​
assertEq(token.balanceOf(newRecovery) - newRecoveryBefore, 140 * ONE);
}

Test result:

[PASS] testOwnerCanChangeRecoveryAddressAfterWithdrawalsAreDisabled()

Recommended Mitigation

Explanation:
Lock recoveryAddress once stakers can no longer withdraw. This keeps sponsor flexibility during setup, but prevents the destination of corrupted-resolution funds from being changed after users are locked into the pool.


Alternatively, the protocol can lock recoveryAddress on the first stake, similar to the existing expiryLocked mechanism.

error RecoveryAddressLocked();
​
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
// Once risk has started, stakers cannot withdraw,
// so the recovery destination should no longer be mutable.
if (riskWindowStart != 0) revert RecoveryAddressLocked();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!