Root + Impact
Description
Pool initialization and setRecoveryAddress() reject only the zero address. A sponsor can therefore select a burn/irrecoverable destination that accepts the full sweep, or a recipient that the stake token refuses to credit, causing every recovery attempt to revert until trusted-owner intervention.
recoveryAddress receives the complete balance on bad-faith CORRUPTED, the remainder after a good-faith bounty window, and unreserved bonus. A safe configuration must therefore avoid destinations that irrecoverably hold tokens and destinations that are incompatible with the selected token's recipient policy.
The contract validates neither property. Initialization and the mutable setter accept any nonzero address, including the pool, the stake token itself, common burn addresses, contracts with no recovery mechanism, and addresses permanently blocked by the token. Each sweep then performs a single safeTransfer() to that live value with no alternate recipient.
src/ConfidencePool.sol
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer {
if (recoveryAddress_ == address(0)) revert InvalidRecoveryAddress();
recoveryAddress = recoveryAddress_;
outcome = PoolStates.Outcome.UNRESOLVED;
_replaceScope(accounts);
_transferOwnership(owner_);
}
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();
stakeToken.safeTransfer(recoveryAddress, toSweep);
emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
}
function sweepUnclaimedCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
stakeToken.safeTransfer(recoveryAddress, amount);
emit UnclaimedCorruptedSwept(msg.sender, recoveryAddress, amount);
}
function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
With a standard ERC20, sending to 0xdead succeeds and permanently removes the full pool from usable custody. With a token that rejects a particular recipient, SafeERC20 propagates the revert; atomicity preserves balances and accounting, but every retry against the same configuration fails. The owner can repair the latter by changing recoveryAddress, so the DoS becomes permanent when that role is unavailable, renounced, compromised, or unwilling to correct the recipient.
Risk
Likelihood:
-
The issue requires trusted-sponsor misconfiguration or compromise, or a token recipient policy that changes after a once-valid address is configured.
-
Standard ERC20 transfers do not call recipient hooks, but allowlisted tokens may still implement blacklists, ERC777/ERC1363-style callbacks, or other recipient restrictions.
Impact:
-
A transfer-accepting burn or irrecoverable destination receives the entire recovery amount successfully, leaving no on-chain retry path.
-
A transfer-rejecting destination blocks all three recovery routes and can strand the pool's full balance indefinitely without owner remediation.
Proof of Concept
Create test/audit/CP014InvalidRecoveryRecipient.t.sol with the following contents:
pragma solidity 0.8.26;
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
contract RecipientBlockingERC20 is ERC20 {
error RecipientBlocked();
address public immutable blockedRecipient;
constructor(address blockedRecipient_) ERC20("Recipient Blocking Token", "RBT") {
blockedRecipient = blockedRecipient_;
}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
function _update(address from, address to, uint256 amount) internal override {
if (to == blockedRecipient) revert RecipientBlocked();
super._update(from, to, amount);
}
}
contract CP014InvalidRecoveryRecipientTest is BaseConfidencePoolTest {
address internal constant DEAD_RECIPIENT = address(0xdead);
address internal constant BLOCKED_RECIPIENT = address(0xB10C);
function test_DeadRecoveryRecipientIrrecoverablyReceivesTheWholePool() external {
_stake(alice, 100 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
pool.setRecoveryAddress(DEAD_RECIPIENT);
pool.claimCorrupted();
assertEq(token.balanceOf(address(pool)), 0);
assertEq(token.balanceOf(DEAD_RECIPIENT), 100 * ONE);
assertEq(token.balanceOf(recovery), 0);
}
function test_BlockedRecoveryRecipientRevertsEverySweep() external {
RecipientBlockingERC20 blockingToken = new RecipientBlockingERC20(BLOCKED_RECIPIENT);
ConfidencePool blockingPool = _deployPoolWithToken(address(blockingToken));
blockingToken.mint(alice, 100 * ONE);
vm.startPrank(alice);
blockingToken.approve(address(blockingPool), 100 * ONE);
blockingPool.stake(100 * ONE);
vm.stopPrank();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
blockingPool.pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
blockingPool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
blockingPool.setRecoveryAddress(BLOCKED_RECIPIENT);
for (uint256 i; i < 2; ++i) {
vm.expectRevert(RecipientBlockingERC20.RecipientBlocked.selector);
blockingPool.claimCorrupted();
}
assertEq(blockingToken.balanceOf(address(blockingPool)), 100 * ONE);
assertEq(blockingPool.corruptedReserve(), 100 * ONE, "reverting transfer rolls back sweep accounting");
assertFalse(blockingPool.claimsStarted());
}
function _deployPoolWithToken(address tokenAddress) internal returns (ConfidencePool deployedPool) {
ConfidencePool implementation = new ConfidencePool();
deployedPool = ConfidencePool(Clones.clone(address(implementation)));
deployedPool.initialize(
agreement,
tokenAddress,
address(safeHarborRegistry),
moderator,
block.timestamp + 31 days,
ONE,
recovery,
address(this),
_defaultScope()
);
}
}
Run the PoC from the repository root:
-
Execute forge test --offline --match-path test/audit/CP014InvalidRecoveryRecipient.t.sol -vv.
-
Confirm that the dead recipient receives all 100 tokens in the first test, while two consecutive recovery attempts revert and leave the full pool balance stranded in the second.
Recommended Mitigation
Reject objectively unsafe destinations at both initialization and update time, including the pool itself, the stake token, and protocol-recognized burn sinks. Route recovery through a governed vault with a tested token policy when arbitrary token behavior must be supported.
src/ConfidencePool.sol
+address internal constant DEAD_ADDRESS = 0x000000000000000000000000000000000000dEaD;
+function _validateRecoveryAddress(address candidate, address token) internal view {
+ if (
+ candidate == address(0) || candidate == address(this) || candidate == token
+ || candidate == DEAD_ADDRESS
+ ) {
+ revert InvalidRecoveryAddress();
+ }
+}
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer {
// ... other validation omitted ...
- if (recoveryAddress_ == address(0)) revert InvalidRecoveryAddress();
+ _validateRecoveryAddress(recoveryAddress_, stakeToken_);
// ... remaining initialization omitted ...
recoveryAddress = recoveryAddress_;
outcome = PoolStates.Outcome.UNRESOLVED;
_replaceScope(accounts);
_transferOwnership(owner_);
}
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner {
- if (newRecoveryAddress == address(0)) revert InvalidRecoveryAddress();
+ _validateRecoveryAddress(newRecoveryAddress, address(stakeToken));
address oldRecoveryAddress = recoveryAddress;
recoveryAddress = newRecoveryAddress;
emit RecoveryAddressUpdated(oldRecoveryAddress, newRecoveryAddress);
}
No static address check can prove future compatibility with a mutable blacklist or callback-bearing token. Keep failed transfers atomic, preserve the ability to rotate a rejected recipient, monitor recovery failures, and document that renouncing ownership also removes that repair path.
