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

`recoveryAddress` has no latch; owner can redirect the whole pool before settlement

Author Revealed upon completion

Root + Impact

Description

  • recoveryAddress is the destination of CORRUPTED settlement: under bad-faith CORRUPTED the entire pool (staker principal + bonus) is transferred to it. To protect staker reliance, expiry is latched once via expiryLocked on the first stake and cannot change afterwards.

  • Unlike expiry, setRecoveryAddress() has no latch, timelock, or post-settlement gate. The owner (sponsor) can change recoveryAddress to any address at any time after stakers deposit — including after the outcome is flagged CORRUPTED and before the sweep executes — redirecting the entire pool to an arbitrary address. The recovery address stakers trusted at deposit time is no longer guaranteed.

// src/ConfidencePool.sol
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
@> // no latch / no timelock / no outcome gate: redirectable any time before the sweep
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
​
function claimCorrupted() external nonReentrant {
...
uint256 toSweep = stakeToken.balanceOf(address(this));
...
@> stakeToken.safeTransfer(recoveryAddress, toSweep); // uses the current (possibly changed) recoveryAddress
}

Risk

Likelihood:

  • The owner key is compromised, or the sponsor changes their mind / turns malicious after receiving staker funds, and calls setRecoveryAddress before the CORRUPTED sweep

  • The pool enters bad-faith CORRUPTED, and claimCorrupted / sweepUnclaimedCorrupted transfer the whole pool to the current recoveryAddress

Impact:

  • Staker principal (under bad-faith CORRUPTED) and bonus are redirected to an address the owner picks after the fact, breaking the trust premise stakers had at deposit time

  • Inconsistent with the one-time expiry latch: both are parameters that determine staker fund outcomes, yet one is latched and the other is freely mutable

Proof of Concept

// test/audit/L04_RecoveryRedirect.t.sol
function testPoC_L04_ownerRedirectsWholePoolAfterCommit() external {
address redirectTarget = makeAddr("ownerControlledEOA");
​
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
assertEq(pool.recoveryAddress(), recovery);
​
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
// still redirectable after the outcome is flagged
pool.setRecoveryAddress(redirectTarget);
​
pool.claimCorrupted();
assertEq(token.balanceOf(redirectTarget), 150 * ONE); // whole pool redirected
assertEq(token.balanceOf(recovery), 0);
}
forge test --offline --match-path test/audit/L04_RecoveryRedirect.t.sol

Recommended Mitigation

function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
+ // Disallow redirection after settlement is finalized, so the whole pool cannot be
+ // repointed to a new address right before the sweep.
+ if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
​
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!