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

`setRecoveryAddress` Has No `outcome == UNRESOLVED` Guard — Owner Can Atomically Redirect CORRUPTED Full-Pool Sweep

Author Revealed upon completion

Title: setRecoveryAddress Has No outcome == UNRESOLVED Guard — Owner Can Atomically Redirect CORRUPTED Full-Pool Sweep

Severity: Low

Description

In ConfidencePool.sol:setRecoveryAddress, the function is onlyOwner with no outcome check, no timelock, and no two-step change:

function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}

All post-resolution sweep functions read recoveryAddress live at transfer time — ConfidencePool.sol:claimCorrupted, ConfidencePool.sol:sweepUnclaimedCorrupted, ConfidencePool.sol:sweepUnclaimedBonus. recoveryAddress is not frozen at resolution. The owner can change recoveryAddress at any moment, including after a CORRUPTED outcome is flagged but before the sweep is called, atomically bundling setRecoveryAddress(newDest) → claimCorrupted() in one transaction to redirect the full-pool sweep before any staker or observer reacts.

Per DESIGN.md, recoveryAddress is the sponsor-controlled CORRUPTED sweep destination, and the sponsor is the economic beneficiary of excess/sweep value — so this is not an unprivileged-attacker extraction. It is a trust-boundary issue: a staker verifying pool parameters at deposit time (DESIGN: "All staker-relevant state is on-chain; stakers should verify pool parameters before depositing") can rely on recoveryAddress only up to the resolution instant, not beyond it.

Contract: ConfidencePool.sol
Function: setRecoveryAddress()

Impact

No third-party loss beyond the sponsor's own value. Stakers lose the ability to predict, at stake time, where residual/sweep value will land if the pool goes CORRUPTED — the binding audit-trail destination is mutable post-deposit and post-flag. Principle of least surprise is violated. The named good-faith attacker (whose bounty destination recoveryAddress only matters when the bounty is unclaimed) is likewise affected when the sponsor changes recoveryAddress mid-claim-window.

Likelihood: Low. Requires the sponsor (a single trusted actor) to act adversarially against their own pool — outside the strict adversarial model (DESIGN) but within the sponsor trust surface (DESIGN) — or to be a compromised key.

Proof of Code

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @title setRecoveryAddress has no outcome gate -- owner atomically redirects CORRUPTED sweep to any address.
contract SetRecoveryAddressNoOutcomeGate is BaseConfidencePoolTest {
using Clones for address;
​
function test_setRecoveryAddressNoOutcomeGate_redirectsCorruptedSweep() 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 thief = makeAddr("thief");
uint256 poolBal = token.balanceOf(address(pool));
​
vm.startPrank(address(this));
pool.setRecoveryAddress(thief);
pool.claimCorrupted();
vm.stopPrank();
​
assertEq(token.balanceOf(thief), poolBal, "sweep redirected");
assertEq(token.balanceOf(recovery), 0);
}
}

Recommended Mitigation

Freeze recoveryAddress at first resolution by snapshotting it:

address public resolutionRecoveryAddress;
...
function flagOutcome(...) {
...
resolutionRecoveryAddress = recoveryAddress;
}

and read resolutionRecoveryAddress in all post-resolution sweep paths. Alternatively, gate the setter:

require(outcome == PoolStates.Outcome.UNRESOLVED, OutcomeAlreadySet());

A two-step address change with a 48-hour accept window adds further defense-in-depth and gives stakers a final verification opportunity.

Updates

Lead Judging Commences

inallhonesty Lead Judge
7 days ago
inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Known Design #10

Support

FAQs

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

Give us feedback!