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

setRecoveryAddress lacks a state guard, allowing the sponsor to redirect staker principal after CORRUPTED is flagged

Author Revealed upon completion

Root + Impact

Description

  • recoveryAddress is the destination for all CORRUPTED-path fund sweeps. Unlike its two sibling sponsor-controlled fields, expiry and scope, which are each frozen by a one-way latch (expiryLocked, scopeLocked), recoveryAddress has no lock at all.

  • The owner can call setRecoveryAddress at any time, including after flagOutcome(CORRUPTED, ...) has fixed the outcome and before claimCorrupted() sweeps the funds — a window where stakers have zero recourse, since withdraw() is already permanently disabled by riskWindowStart != 0.

// Root cause in the codebase with @> marks to highlight the relevant section
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner
{
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
@> // no outcome check, no claimsStarted check — unlike setExpiry/setPoolScope
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
function claimCorrupted() external nonReentrant
{
...
@> stakeToken.safeTransfer(recoveryAddress, toSweep); // reads live storage, not a snapshot
}

Risk

Likelihood:

  • Occurs whenever the owner front-runs their own claimCorrupted() call with setRecoveryAddress, a timing entirely within the owner's control.


Applies to both good-faith and bad-faith CORRUPTED resolutions, since both eventually route through recoveryAddress.


Impact:

  • Full staker principal and bonus redirected to an owner-controlled address at the exact point stakers can no longer withdraw.



Proof of Concept:

The PoC stakes Alice, resolves the pool bad-faith CORRUPTED, confirms Alice's withdraw() now reverts (proving she's locked in), then has the owner call setRecoveryAddress to a new wallet before anyone calls claimCorrupted(). It asserts the original recovery address received nothing and the attacker-controlled wallet received Alice's full 100 tokens. A second test stakes and withdraws once, then shows setExpiry and setPoolScope both correctly revert once locked, while setRecoveryAddress succeeds unconditionally in the same state — isolating the missing guard as the only asymmetry among the three sponsor-controlled fields.

function test_PoC_SponsorHijacksRecoveryAddressAfterCorruptedFlag() external
{
_stake(alice, 100 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
vm.prank(alice);
vm.expectRevert(IConfidencePool.WithdrawsDisabled.selector);
pool.withdraw();
​
address attackerWallet = makeAddr("attackerWallet");
pool.setRecoveryAddress(attackerWallet); // succeeds, no guard
​
uint256 recoveryBefore = token.balanceOf(recovery);
pool.claimCorrupted();
​
assertEq(token.balanceOf(recovery) - recoveryBefore, 0);
assertEq(token.balanceOf(attackerWallet), 100 * ONE);
}

Recommended Mitigation

Add the same one-way guard already used for expiry and scope: once CORRUPTED is flagged, freeze recoveryAddress. This closes the redirection window without restricting the field before an outcome is set, which is when a sponsor may legitimately need to correct it.

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

Support

FAQs

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

Give us feedback!