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

Unlocked recoveryAddress lets the sponsor redirect a confirmed CORRUPTED sweep after the fact

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: expiry and pool scope are sponsor-mutable only up to a defined checkpoint (expiryLocked, scopeLocked), so stakers can rely on both being fixed once risk materializes.

  • The issue: recoveryAddress — which receives 100% of staker principal + bonus on a bad-faith CORRUPTED resolution — has no equivalent lock, at any point, including after the breach is confirmed.

function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
@> // no check on `outcome` / `claimsStarted` — callable at any lifecycle stage
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(recoveryAddress, newRecoveryAddress);
}
​
// claimCorrupted() sweeps the CORRUPTED-path balance to whatever
// `recoveryAddress` currently holds, read live at call time:
@> stakeToken.safeTransfer(recoveryAddress, toSweep);

Risk

Likelihood:

  • The owner keeps unrestricted onlyOwner access to setRecoveryAddress for the pool's entire life — unlike expiry/scope, which the same codebase already freezes once stakers rely on them.

  • flagOutcome(CORRUPTED, false, address(0)) publicly commits the pool's full balance to the current recoveryAddress, giving the owner an exact on-chain moment to front-run the sweep with one extra call.

Impact:

  • 100% of staked principal + bonus lands in an address of the owner's choosing instead of the address stakers deposited against.

  • The window never closes — it persists through claimsStarted, covering partial-bounty remainders and later donation sweeps too.

Proof of Concept

The test below reproduces the full attack in four steps:

  1. Alice and Bob stake 150 total; Carol contributes 20 bonus — 170 tokens are now at stake.

  2. A real attack is observed and confirmed: the registry reports CORRUPTED, and the moderator correctly flags it bad-faith (flagOutcome(CORRUPTED, false, address(0))). At this point corruptedReserve locks in at 170, publicly owed to the current recoveryAddress.

  3. Only after that confirmation, the owner calls setRecoveryAddress(rugWallet) — the call succeeds because nothing gates it on the now-CORRUPTED outcome.

  4. Anyone (the owner or a third party) calls the permissionless claimCorrupted(). It reads recoveryAddress fresh at execution time, so the full 170 tokens pay rugWallet, and the address stakers actually trusted receives zero.

// test/unit/ConfidencePool.recoveryAddressRug.t.sol
function testRecoveryAddressRugPullAfterBadFaithCorrupted() external {
_stake(alice, 100 * ONE);
_stake(bob, 50 * ONE);
_contributeBonus(carol, 20 * ONE);
​
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
assertEq(pool.corruptedReserve(), 170 * ONE);
​
@> pool.setRecoveryAddress(rugWallet); // owner redirects post-confirmation
pool.claimCorrupted(); // permissionless sweep, pays rugWallet
​
assertEq(token.balanceOf(rugWallet), 170 * ONE);
assertEq(token.balanceOf(recovery), 0); // published address gets nothing
}

Verified locally: forge test --match-test testRecoveryAddressRugPullAfterBadFaithCorrupted → 1 passed.

Recommended Mitigation

function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
+ if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}

outcome is the exact moment corruptedReserve/bountyEntitlement get committed against the current recoveryAddress (see flagOutcome), so gating on it closes the window at its source rather than patching a symptom. It costs nothing functionally: the sponsor keeps full flexibility to update recoveryAddress at any time before an outcome is flagged — which is the only window where staker funds aren't yet at stake — and reuses the existing OutcomeAlreadySet error, so the fix is a single line with no new interface surface or storage.

Support

FAQs

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

Give us feedback!