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

Recovery destination remains owner-mutable after resolution

Author Revealed upon completion

Root + Impact

Description

recoveryAddress can be changed by the pool owner at any time, including after a terminal outcome has fixed the economic allocation. claimCorrupted(), sweepUnclaimedCorrupted(), and sweepUnclaimedBonus() read the live storage value when they transfer funds rather than a recipient captured at resolution.

A compromised or malicious owner can therefore wait until users and keepers observe a settled CORRUPTED, SURVIVED, or EXPIRED pool, replace the recipient, and redirect the pending sweep. In the bad-faith CORRUPTED case, this redirects the entire pool balance, including all staker principal and bonus.

The setter is restricted only by owner authentication and a zero-address check:

src/ConfidencePool.sol
​
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress; // @> No unresolved/outcome/claims guard applies.
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}

No resolution function stores the then-current recipient. Each sweep dereferences the mutable value immediately before the external transfer:

function claimCorrupted() external nonReentrant {
// ... outcome checks and accounting ...
stakeToken.safeTransfer(recoveryAddress, toSweep); // @> Live recipient gets the full pool.
emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
}
​
function sweepUnclaimedCorrupted() external nonReentrant {
// ... deadline checks and accounting ...
stakeToken.safeTransfer(recoveryAddress, amount); // @> Live recipient is read after settlement.
emit UnclaimedCorruptedSwept(msg.sender, recoveryAddress, amount);
}
​
function sweepUnclaimedBonus() external nonReentrant {
// ... reserve calculation ...
stakeToken.safeTransfer(recoveryAddress, amount); // @> SURVIVED/EXPIRED excess is also redirectable.
emit BonusSwept(msg.sender, recoveryAddress, amount);
}

This creates a transaction-ordering surface. A keeper can submit a permissionless sweep to the public mempool; the owner can front-run it with setRecoveryAddress(attacker), causing the keeper's otherwise valid transaction to execute against the replacement. No additional confirmation, delay, or participant veto exists.

The owner is an explicit trusted sponsor under the design, which bounds the likelihood. However, post-resolution mutability is stronger than ordinary pre-funding configuration authority because it can alter the destination after the outcome and recoverable amount are already known. Ownable2Step protects ownership transfers but does not mitigate compromise of the accepted owner.

Risk

Likelihood:

  • Only the pool owner can change the address, so exploitation requires sponsor malice, key compromise, or an operational mistake.

  • The path is available in every lifecycle state and can be executed immediately before any permissionless sweep.

  • High-value settled pools and public sweep transactions make the redirection opportunity easy to identify once owner authority is compromised.

Impact:

  • In bad-faith CORRUPTED pools, all principal and bonus can be redirected from the recipient users observed at resolution.

  • In good-faith CORRUPTED and SURVIVED/EXPIRED pools, remaining bounty, unclaimed bonus, dust, and donations can be redirected.

  • Events emitted by the original resolution do not commit a recipient, so off-chain expectations cannot enforce the earlier value.

Proof of Concept

Create test/audit/CP054MutableRecoveryAfterResolution.t.sol with the following contents:

// 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 CP054MutableRecoveryAfterResolutionTest is BaseConfidencePoolTest {
function test_OwnerRedirectsSettledCorruptedPoolBeforeSweep() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
address recoveryAtResolution = pool.recoveryAddress();
address replacementRecovery = makeAddr("replacementRecovery");
assertEq(recoveryAtResolution, recovery);
​
pool.setRecoveryAddress(replacementRecovery);
assertEq(pool.recoveryAddress(), replacementRecovery);
​
uint256 originalBefore = token.balanceOf(recoveryAtResolution);
uint256 replacementBefore = token.balanceOf(replacementRecovery);
pool.claimCorrupted();
​
assertEq(token.balanceOf(recoveryAtResolution) - originalBefore, 0, "settled recipient receives nothing");
assertEq(
token.balanceOf(replacementRecovery) - replacementBefore,
150 * ONE,
"live replacement receives principal and bonus"
);
assertEq(token.balanceOf(address(pool)), 0);
}
}

Run the PoC from the repository root:

  1. Execute forge test --offline --match-path test/audit/CP054MutableRecoveryAfterResolution.t.sol -vv.

  2. Confirm that the test passes. The pool is settled as bad-faith CORRUPTED with a 150-token balance, the owner changes the recovery address afterward, and the full balance goes to the replacement while the original recipient receives nothing.

Recommended Mitigation

Snapshot the recovery recipient whenever an outcome is first resolved and use the snapshot for all value associated with that settlement. Also prevent the ordinary setter from changing settlement routing after resolution. If a broken or blacklisted recipient must remain repairable, expose a separate timelocked governance process with explicit old/new recipient events and sufficient delay for monitoring and cancellation.

src/ConfidencePool.sol
​
+address public resolutionRecoveryAddress;
​
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_)
external onlyModerator
{
// ... validation and snapshots ...
+ resolutionRecoveryAddress = recoveryAddress;
outcome = newOutcome;
// ...
}
​
function claimExpired() external nonReentrant {
// ...
if (outcome == PoolStates.Outcome.UNRESOLVED) {
+ resolutionRecoveryAddress = recoveryAddress;
// ... resolution ...
}
// ...
}
​
function claimCorrupted() external nonReentrant {
// ...
- stakeToken.safeTransfer(recoveryAddress, toSweep);
- emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
+ address recipient = resolutionRecoveryAddress;
+ stakeToken.safeTransfer(recipient, toSweep);
+ emit ClaimCorrupted(msg.sender, recipient, toSweep);
}
​
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ if (outcome != PoolStates.Outcome.UNRESOLVED) revert RecoveryAddressLocked();
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
// ...
}

Apply the same snapshot to both unclaimed-corrupted and bonus sweeps. Snapshot updates during an allowed pre-claim moderator re-flag should follow a documented rule; the safest approach is to retain the recipient captured by the first terminal outcome unless governance deliberately executes the delayed recovery-repair process.

Support

FAQs

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

Give us feedback!