Root + Impact
Description
The pool accepts its own address as recoveryAddress. Transfers to that address succeed for a standard ERC-20 without reducing the pool balance, while the sweep functions still finalize accounting and emit successful-sweep events.
Recovery sweeps are intended to remove tokens from the pool and deliver them to the sponsor-selected recoveryAddress. Both initialization and setRecoveryAddress() reject only the zero address, so the configured recipient can be the pool itself.
For a standard ERC-20, transferring from the pool to itself debits and credits the same account. The call succeeds, but the pool's token balance does not change. claimCorrupted() nevertheless decrements corruptedReserve, latches claimsStarted, and emits ClaimCorrupted. sweepUnclaimedCorrupted() similarly clears the corrupted reserve and bounty accounting. In the bonus path, sweepUnclaimedBonus() can decrement totalBonus and emit BonusSwept even though no tokens moved; because the balance is unchanged, the same nominal amount remains permissionlessly sweepable on later calls.
src/ConfidencePool.sol
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();
corruptedReserve = toSweep <= corruptedReserve ? corruptedReserve - toSweep : 0;
stakeToken.safeTransfer(recoveryAddress, toSweep);
emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
}
function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
The CORRUPTED sweep functions size subsequent calls from the live pool balance, so an available owner can repair the configuration and sweep again. Until that happens, however, the entire recoverable balance remains trapped behind successful-looking state and events; it becomes permanently inaccessible if the owner is unavailable.
Risk
Likelihood:
-
The initial pool creator or current pool owner must deliberately or accidentally select the pool's address. No unprivileged caller can change recoveryAddress.
-
Once configured, any permissionless recovery sweep reaches the issue with an ordinary standards-compliant ERC-20; no malicious token behavior is required.
Impact:
-
A CORRUPTED resolution can appear settled while the full principal and bonus balance remains in the clone and the intended recovery recipient receives nothing.
-
Reserve, bounty, or bonus bookkeeping and emitted events can claim a successful sweep despite an unchanged balance, and bonus self-sweeps can be repeated permissionlessly.
-
Rotating to a valid recipient recovers the balance, but loss of owner availability turns the configuration error into a permanent lock.
Proof of Concept
Create test/audit/CP013RecoveryAddressSelfSweep.t.sol with the following contents:
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
contract CP013RecoveryAddressSelfSweepTest is BaseConfidencePoolTest {
function test_SelfRecoveryMakesCorruptedSweepSucceedWithoutMovingFunds() 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));
pool.setRecoveryAddress(address(pool));
uint256 poolBalance = token.balanceOf(address(pool));
assertEq(poolBalance, 150 * ONE);
assertEq(pool.corruptedReserve(), poolBalance);
vm.expectEmit(true, true, false, true, address(pool));
emit IConfidencePool.ClaimCorrupted(address(this), address(pool), poolBalance);
pool.claimCorrupted();
assertEq(token.balanceOf(address(pool)), poolBalance, "the self-transfer moves no tokens");
assertEq(pool.corruptedReserve(), 0, "accounting says the full reserve was swept");
assertTrue(pool.claimsStarted(), "the outcome is latched despite no fund movement");
pool.setRecoveryAddress(recovery);
uint256 recoveryBefore = token.balanceOf(recovery);
pool.claimCorrupted();
assertEq(token.balanceOf(recovery) - recoveryBefore, poolBalance);
assertEq(token.balanceOf(address(pool)), 0);
}
function test_SelfRecoveryMakesBonusSweepEraseAccountingWithoutMovingFunds() external {
_contributeBonus(carol, 40 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
pool.setRecoveryAddress(address(pool));
uint256 poolBalance = token.balanceOf(address(pool));
assertEq(pool.totalBonus(), poolBalance);
vm.expectEmit(true, true, false, true, address(pool));
emit IConfidencePool.BonusSwept(bob, address(pool), poolBalance);
vm.prank(bob);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(address(pool)), poolBalance, "the swept bonus remains in the pool");
assertEq(pool.totalBonus(), 0, "bonus accounting is cleared despite the unchanged balance");
vm.prank(alice);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(address(pool)), poolBalance);
pool.setRecoveryAddress(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery), poolBalance);
assertEq(token.balanceOf(address(pool)), 0);
}
}
Run the PoC from the repository root:
-
Execute forge test --offline --match-path test/audit/CP013RecoveryAddressSelfSweep.t.sol -vv.
-
Confirm that both tests pass and the suite reports two tests passed with zero failures.
Recommended Mitigation
Reject the pool itself wherever the recovery address is accepted. The initializer check is required because the factory can pass the clone's predictable address before ownership-based setters are available.
src/ConfidencePool.sol
function initialize(
address agreement_,
// ... unchanged parameters omitted ...
address[] calldata accounts
) external initializer {
// ... unchanged validation omitted ...
- if (recoveryAddress_ == address(0)) revert InvalidRecoveryAddress();
+ if (recoveryAddress_ == address(0) || recoveryAddress_ == address(this)) {
+ revert InvalidRecoveryAddress();
+ }
if (expiry_ < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
// ... unchanged initialization omitted ...
_transferOwnership(owner_);
}
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
- if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
+ if (newRecoveryAddress == address(0) || newRecoveryAddress == address(this)) {
+ revert InvalidRecoveryAddress();
+ }
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
