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

Sponsor can front-run a terminal outcome by redirecting recoveryAddress before the CORRUPTED/unclaimed-bonus sweep executes

Author Revealed upon completion

Description

ConfidencePool allows the pool owner to update recoveryAddress via setRecoveryAddress with no restriction tied to the pool's resolution state — unlike expiry (locked after first stake) and pool scope (locked once the registry leaves pre-attack staging), both enforced as one-way latches on-chain.

Three sweep functions read recoveryAddress live from storage at call time: claimCorrupted (L423), sweepUnclaimedCorrupted (L468), and sweepUnclaimedBonus (L506). Because there is no lock, the owner can call setRecoveryAddress at any point up to the exact block before a sweep executes — even after the moderator has already flagged a terminal outcome (SURVIVED, CORRUPTED, or EXPIRED).

A staker or observer who checks recoveryAddress pre-resolution (e.g. confirming it points to a legitimate treasury multisig) has no on-chain guarantee the same address receives the swept funds once resolution occurs.

Root Cause: setRecoveryAddress (pre-patch) had no check against outcome, unlike expiry (locked at first stake, see test_expiryLock_setOnFirstStake) and scope (locked on first post-staging interaction, see testScopeLockedEventEmittedOnceAndNotAgain). Three separate sweep sites read recoveryAddress live from storage at call time (claimCorrupted L423, sweepUnclaimedCorrupted L468, sweepUnclaimedBonus L506), so a sponsor could redirect the destination in the window between flagOutcome reaching a terminal state and any of these sweeps executing.

/// @inheritdoc IConfidencePool
// aderyn-ignore-next-line(centralization-risk)
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
@> // @audit no check against `outcome` — recoveryAddress can be changed even after
@> // the pool has reached a terminal state (SURVIVED / CORRUPTED / EXPIRED),
@> // right up until the moment a sweep function reads it.
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}

Risk

  • The pool owner (sponsor) calls setRecoveryAddress in the same transaction window between the moderator's flagOutcome(CORRUPTED) call and any staker or the owner themselves calling claimCorrupted/sweepUnclaimedCorrupted/sweepUnclaimedBonus.

  • Stakers and third-party observers who check recoveryAddress before the pool resolves have no on-chain guarantee that the same address will receive swept funds once resolution actually happens.

Impact:

  • Funds intended for a specific recovery destination (e.g. a treasury multisig disclosed to stakers at deposit time) can be redirected to an address the sponsor controls, with zero timelock or on-chain signal to affected parties.

  • Breaks the trust assumption stakers rely on when evaluating a pool's legitimacy pre-stake, since expiry and scope are both explicitly locked at defined points in the lifecycle but recoveryAddress was not.

Proof of Concept
This test passed against the unpatched contract (confirmed via forge test -vvvv), demonstrating the full pool balance (1500 * ONE tokens in this scenario) landing on the sponsor-controlled colluder address rather than the originally configured recoveryAddress.

## Proof of Concept
​
The following test passes against the unpatched contract (verified via `forge test -vvvv`), showing the full pool balance (1500 * ONE) landing on a sponsor-controlled address instead of the originally configured `recoveryAddress`, after a terminal CORRUPTED flag has already been recorded.
​
​```solidity
function testOwnerRedirectsRecoveryAddressAfterCorruptedFlagBeforeSweep() external {
_stake(alice, 1000 * ONE);
_stake(bob, 500 * 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 colluder = makeAddr("sponsorColluder");
vm.prank(pool.owner());
pool.setRecoveryAddress(colluder); // succeeds despite terminal outcome already set
​
assertEq(pool.recoveryAddress(), colluder);
​
uint256 before = token.balanceOf(colluder);
pool.claimCorrupted();
uint256 swept = token.balanceOf(colluder) - before;
​
assertGt(swept, 0, "full pool balance swept to attacker-controlled address");
assertEq(token.balanceOf(originalRecovery), 0, "original recovery address received nothing");
}
​```
​
**Trace summary:** `RecoveryAddressUpdated` fires *after* `OutcomeFlagged(outcome: CORRUPTED)`. The subsequent `claimCorrupted()` call transfers the entire swept balance to the redirected address; the original `recoveryAddress` receives nothing.

Recommended Mitigation

## Recommended Mitigation
​
Gate `setRecoveryAddress` on the pool having reached any terminal outcome, using the existing `PoolStates.isTerminal` helper. This mirrors the one-way-latch pattern the codebase already applies to `scopeLocked`, so the fix is consistent with the contract's own design idiom rather than a bolt-on restriction. Once `outcome` is no longer `UNRESOLVED`, the sweep destination can no longer be changed — closing the front-running window while leaving legitimate pre-resolution updates (e.g. fixing a typo before any attack occurs) unaffected.
​
​```diff
+ error PoolState__CannotModifyParametersPostResolution();
+
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ if (PoolStates.isTerminal(outcome)) {
+ revert PoolState__CannotModifyParametersPostResolution();
+ }
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
​```
​
**Verification:** patched contract passes a new regression test confirming the redirect now reverts with `PoolState__CannotModifyParametersPostResolution`, plus a second test confirming legitimate pre-resolution updates still succeed. Full existing suite (258 tests) still passes post-patch with no regressions.

Support

FAQs

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

Give us feedback!