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

Blacklisted `recoveryAddress` combined with an unavailable owner permanently freezes the entire pool balance

Author Revealed upon completion

Description

  • Bad-faith CORRUPTED has exactly one payout path: claimCorrupted() sweeps the pool's full token balance to recoveryAddress. Every other exit path in this contract that depends on a single privileged role has a permissionless backstop for when that role becomes unavailable — the moderator has the MODERATOR_CORRUPTED_GRACE (180-day) fallback baked into claimExpired.

  • claimCorrupted() has no equivalent backstop. If recoveryAddress becomes unable to receive the stake token (a compliance blacklist action on a USDC/USDT-style token is a plausible, real-world behavior for any contract accepting arbitrary allowlisted ERC20s) at the same time the pool owner is permanently unavailable (renounced ownership, lost key, compromised and abandoned), no address can ever call setRecoveryAddress again and no other function can move the frozen balance out.

function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (goodFaith && bountyClaimed < bountyEntitlement) revert MustClaimBountyFirst();
​
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();
​
corruptedReserve = toSweep <= corruptedReserve ? corruptedReserve - toSweep : 0;
if (!goodFaith) {
bountyClaimed = bountyEntitlement;
}
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(recoveryAddress, toSweep); //@> sole payout path, no fallback recipient — if this reverts, every future call reverts identically forever
emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
}
​
function setRecoveryAddress(address newRecoveryAddress) external onlyOwner { //@> once owner() == address(0), NO address can ever satisfy onlyOwner again — permanently uncallable
...
}

Risk

Likelihood:

  • Compliance blacklisting is an established, ongoing behavior of the exact class of tokens (USDC, USDT) that a real deployment's stake-token allowlist would plausibly include, and the factory owner's allowlist review has no way to guarantee a currently-clean recoveryAddress stays unblacklisted for the pool's entire lifetime, including well after expiry when a bad-faith CORRUPTED verdict might land.

  • Ownership renunciation is a standard, one-call, irreversible action already exposed by the inherited Ownable2Step (renounceOwnership), and key loss/compromise-then-abandonment is the same permanent-unavailability failure mode this codebase already treats as a first-class risk for the moderator role (hence MODERATOR_CORRUPTED_GRACE) — the owner role carries the identical risk with no equivalent mitigation.

Impact:

  • The entire pool balance (snapshotTotalStaked + snapshotTotalBonus, confirmed at 1500e18 in the PoC) is frozen in the contract permanently, unrecoverable by any staker, the named attacker (bad-faith CORRUPTED has none), or any other party — every future claimCorrupted() call reverts identically forever.

  • Unlike the moderator-unavailability case, which the contract explicitly designs around with a 180-day grace fallback, there is no time-bounded or permissionless recovery mechanism for this failure mode at all.

Proof of Concept

// 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 {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {MockBlacklistableERC20} from "test/mocks/MockBlacklistableERC20.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
​
contract PoC_RecoveryAddressBlacklistTest is Test {
uint256 internal constant ONE = 1e18;
address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);
​
MockBlacklistableERC20 internal token;
MockAttackRegistry internal attackRegistry;
MockSafeHarborRegistry internal safeHarborRegistry;
MockAgreement internal agreementContract;
ConfidencePool internal pool;
​
address internal agreement;
address internal moderator = makeAddr("moderator");
address internal recovery = makeAddr("recovery");
address internal alice = makeAddr("alice");
address internal bob = makeAddr("bob");
​
function setUp() public {
token = new MockBlacklistableERC20();
attackRegistry = new MockAttackRegistry();
safeHarborRegistry = new MockSafeHarborRegistry();
agreementContract = new MockAgreement(address(this));
agreement = address(agreementContract);
agreementContract.setContractInScope(SCOPE_ACCOUNT, 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)));
​
address[] memory scope = new address[](1);
scope[0] = SCOPE_ACCOUNT;
​
pool.initialize(
agreement, address(token), address(safeHarborRegistry), moderator, block.timestamp + 31 days, ONE,
recovery, address(this), scope
);
}
​
function _stake(address user, uint256 amount) internal {
token.mint(user, amount);
vm.startPrank(user);
token.approve(address(pool), amount);
pool.stake(amount);
vm.stopPrank();
}
​
function _contributeBonus(address user, uint256 amount) internal {
token.mint(user, amount);
vm.startPrank(user);
token.approve(address(pool), amount);
pool.contributeBonus(amount);
vm.stopPrank();
}
​
function testPoC_BlacklistedRecoveryAddressPlusRenouncedOwnerPermanentlyFreezesFunds() external {
uint256 stakeAmt = 1_000e18;
uint256 bonusAmt = 500e18;
​
_stake(alice, stakeAmt);
_contributeBonus(bob, bonusAmt);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
assertEq(uint8(pool.outcome()), uint8(PoolStates.Outcome.CORRUPTED));
​
// recoveryAddress becomes unable to receive the token AFTER the outcome is locked in.
token.setBlacklisted(recovery, true);
​
pool.renounceOwnership(); //@> removes the only address that could ever call setRecoveryAddress again
assertEq(pool.owner(), address(0), "owner is permanently gone");
​
vm.expectRevert(
abi.encodeWithSelector(MockBlacklistableERC20.BlacklistedRecipient.selector, recovery)
);
pool.claimCorrupted(); //@> bad-faith CORRUPTED's only payout path — now reverts forever
​
// Repeated attempts, from any caller, always fail the same way.
vm.prank(alice);
vm.expectRevert(
abi.encodeWithSelector(MockBlacklistableERC20.BlacklistedRecipient.selector, recovery)
);
pool.claimCorrupted();
​
assertEq(token.balanceOf(address(pool)), stakeAmt + bonusAmt, "the full pool is permanently stuck"); //@> proof: funds are frozen, unreachable by any party
}
}

Run: forge test --match-path "test/RecoveryAddressBlacklistPermanentLock.t.sol" -vvvv → PASS (gas: 579614)

No files changed, compilation skipped
​
Ran 1 test for test/RecoveryAddressBlacklistPermanentLock.t.sol:PoC_RecoveryAddressBlacklistTest
[PASS] testPoC_BlacklistedRecoveryAddressPlusRenouncedOwnerPermanentlyFreezesFunds() (gas: 579614)
Traces:
[664014] PoC_RecoveryAddressBlacklistTest::testPoC_BlacklistedRecoveryAddressPlusRenouncedOwnerPermanentlyFreezesFunds()
├─ [48648] MockBlacklistableERC20::mint(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], 1000000000000000000000 [1e21])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], value: 1000000000000000000000 [1e21])
│ └─ ← [Stop]
├─ [0] VM::startPrank(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6])
│ └─ ← [Return]
├─ [24325] MockBlacklistableERC20::approve(0xa0Cb889707d426A7A386870A03bc70d1b0697598, 1000000000000000000000 [1e21])
│ ├─ emit Approval(owner: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], spender: 0xa0Cb889707d426A7A386870A03bc70d1b0697598, value: 1000000000000000000000 [1e21])
│ └─ ← [Return] true
├─ [240244] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::stake(1000000000000000000000 [1e21])
│ ├─ [237575] ConfidencePool::stake(1000000000000000000000 [1e21]) [delegatecall]
│ │ ├─ [2323] MockSafeHarborRegistry::getAttackRegistry() [staticcall]
│ │ │ └─ ← [Return] MockAttackRegistry: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
│ │ ├─ [2374] MockAttackRegistry::getAgreementState(MockAgreement: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9]) [staticcall]
│ │ │ └─ ← [Return] 1
│ │ ├─ [2537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ │ │ └─ ← [Return] 0
│ │ ├─ [25606] MockBlacklistableERC20::transferFrom(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], 0xa0Cb889707d426A7A386870A03bc70d1b0697598, 1000000000000000000000 [1e21])
│ │ │ ├─ emit Transfer(from: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], to: 0xa0Cb889707d426A7A386870A03bc70d1b0697598, value: 1000000000000000000000 [1e21])
│ │ │ └─ ← [Return] true
│ │ ├─ [537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ │ │ └─ ← [Return] 1000000000000000000000 [1e21]
│ │ ├─ emit Staked(staker: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], amount: 1000000000000000000000 [1e21])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [26748] MockBlacklistableERC20::mint(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 500000000000000000000 [5e20])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], value: 500000000000000000000 [5e20])
│ └─ ← [Stop]
├─ [0] VM::startPrank(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e])
│ └─ ← [Return]
├─ [24325] MockBlacklistableERC20::approve(0xa0Cb889707d426A7A386870A03bc70d1b0697598, 500000000000000000000 [5e20])
│ ├─ emit Approval(owner: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], spender: 0xa0Cb889707d426A7A386870A03bc70d1b0697598, value: 500000000000000000000 [5e20])
│ └─ ← [Return] true
├─ [34820] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::contributeBonus(500000000000000000000 [5e20])
│ ├─ [34651] ConfidencePool::contributeBonus(500000000000000000000 [5e20]) [delegatecall]
│ │ ├─ [323] MockSafeHarborRegistry::getAttackRegistry() [staticcall]
│ │ │ └─ ← [Return] MockAttackRegistry: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
│ │ ├─ [374] MockAttackRegistry::getAgreementState(MockAgreement: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9]) [staticcall]
│ │ │ └─ ← [Return] 1
│ │ ├─ [537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ │ │ └─ ← [Return] 1000000000000000000000 [1e21]
│ │ ├─ [3706] MockBlacklistableERC20::transferFrom(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 0xa0Cb889707d426A7A386870A03bc70d1b0697598, 500000000000000000000 [5e20])
│ │ │ ├─ emit Transfer(from: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], to: 0xa0Cb889707d426A7A386870A03bc70d1b0697598, value: 500000000000000000000 [5e20])
│ │ │ └─ ← [Return] true
│ │ ├─ [537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ │ │ └─ ← [Return] 1500000000000000000000 [1.5e21]
│ │ ├─ emit BonusContributed(contributor: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], amount: 500000000000000000000 [5e20])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [3254] MockAttackRegistry::setAgreementState(3)
│ └─ ← [Return]
├─ [26481] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::pokeRiskWindow()
│ ├─ [26318] ConfidencePool::pokeRiskWindow() [delegatecall]
│ │ ├─ [323] MockSafeHarborRegistry::getAttackRegistry() [staticcall]
│ │ │ └─ ← [Return] MockAttackRegistry: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
│ │ ├─ [374] MockAttackRegistry::getAgreementState(MockAgreement: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9]) [staticcall]
│ │ │ └─ ← [Return] 3
│ │ ├─ emit ScopeLocked(timestamp: 1)
│ │ ├─ emit RiskWindowStarted(timestamp: 1)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [454] MockAttackRegistry::setAgreementState(6)
│ └─ ← [Return]
├─ [0] VM::prank(moderator: [0x62853092D02FA98524B9618334F605a1801432cA])
│ └─ ← [Return]
├─ [147446] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::flagOutcome(2, false, 0x0000000000000000000000000000000000000000)
│ ├─ [147265] ConfidencePool::flagOutcome(2, false, 0x0000000000000000000000000000000000000000) [delegatecall]
│ │ ├─ [323] MockSafeHarborRegistry::getAttackRegistry() [staticcall]
│ │ │ └─ ← [Return] MockAttackRegistry: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
│ │ ├─ [374] MockAttackRegistry::getAgreementState(MockAgreement: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9]) [staticcall]
│ │ │ └─ ← [Return] 6
│ │ ├─ emit RiskWindowEnded(timestamp: 1)
│ │ ├─ emit OutcomeFlagged(moderator: moderator: [0x62853092D02FA98524B9618334F605a1801432cA], outcome: 2, goodFaith: false, attacker: 0x0000000000000000000000000000000000000000)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [632] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::outcome() [staticcall]
│ ├─ [466] ConfidencePool::outcome() [delegatecall]
│ │ └─ ← [Return] 2
│ └─ ← [Return] 2
├─ [22655] MockBlacklistableERC20::setBlacklisted(recovery: [0x73030B99950fB19C6A813465E58A0BcA5487FBEa], true)
│ └─ ← [Return]
├─ [7870] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::renounceOwnership()
│ ├─ [7707] ConfidencePool::renounceOwnership() [delegatecall]
│ │ ├─ emit OwnershipTransferred(previousOwner: PoC_RecoveryAddressBlacklistTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], newOwner: 0x0000000000000000000000000000000000000000)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [1295] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::owner() [staticcall]
│ ├─ [1129] ConfidencePool::owner() [delegatecall]
│ │ └─ ← [Return] 0x0000000000000000000000000000000000000000
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ [0] VM::expectRevert(custom error 0xf28dceb3: 000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000241231cfa300000000000000000000000073030b99950fb19c6a813465e58a0bca5487fbea00000000000000000000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [8257] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::claimCorrupted() // <-- the ONLY payout path for bad-faith CORRUPTED reverts, permanently
│ ├─ [8086] ConfidencePool::claimCorrupted() [delegatecall]
│ │ ├─ [537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ │ │ └─ ← [Return] 1500000000000000000000 [1.5e21]
│ │ ├─ [697] MockBlacklistableERC20::transfer(recovery: [0x73030B99950fB19C6A813465E58A0BcA5487FBEa], 1500000000000000000000 [1.5e21])
│ │ │ └─ ← [Revert] BlacklistedRecipient(0x73030B99950fB19C6A813465E58A0BcA5487FBEa)
│ │ └─ ← [Revert] BlacklistedRecipient(0x73030B99950fB19C6A813465E58A0BcA5487FBEa)
│ └─ ← [Revert] BlacklistedRecipient(0x73030B99950fB19C6A813465E58A0BcA5487FBEa)
├─ [0] VM::prank(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf28dceb3: 000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000241231cfa300000000000000000000000073030b99950fb19c6a813465e58a0bca5487fbea00000000000000000000000000000000000000000000000000000000)
│ └─ ← [Return]
├─ [8257] 0xa0Cb889707d426A7A386870A03bc70d1b0697598::claimCorrupted()
│ ├─ [8086] ConfidencePool::claimCorrupted() [delegatecall]
│ │ ├─ [537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ │ │ └─ ← [Return] 1500000000000000000000 [1.5e21]
│ │ ├─ [697] MockBlacklistableERC20::transfer(recovery: [0x73030B99950fB19C6A813465E58A0BcA5487FBEa], 1500000000000000000000 [1.5e21])
│ │ │ └─ ← [Revert] BlacklistedRecipient(0x73030B99950fB19C6A813465E58A0BcA5487FBEa)
│ │ └─ ← [Revert] BlacklistedRecipient(0x73030B99950fB19C6A813465E58A0BcA5487FBEa)
│ └─ ← [Revert] BlacklistedRecipient(0x73030B99950fB19C6A813465E58A0BcA5487FBEa)
├─ [537] MockBlacklistableERC20::balanceOf(0xa0Cb889707d426A7A386870A03bc70d1b0697598) [staticcall]
│ └─ ← [Return] 1500000000000000000000 [1.5e21] // <-- the full 1500e18 remains permanently stuck in the contract, no path out
└─ ← [Return]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.38ms (390.15µs CPU time)
​
Ran 1 test suite in 9.73ms (1.38ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Switch claimCorrupted's sole payout from a push safeTransfer to a pull-payment fallback: attempt the transfer, and if it reverts (blacklist, or any other reason recoveryAddress can't receive), credit the amount to a queued balance instead of reverting the whole call. This removes the single point of failure — one bad recoveryAddress no longer permanently bricks fund recovery for every future caller — while still paying out immediately in the common case.

function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (goodFaith && bountyClaimed < bountyEntitlement) revert MustClaimBountyFirst();
​
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();
​
corruptedReserve = toSweep <= corruptedReserve ? corruptedReserve - toSweep : 0;
if (!goodFaith) {
bountyClaimed = bountyEntitlement;
}
if (!claimsStarted) claimsStarted = true;
- stakeToken.safeTransfer(recoveryAddress, toSweep);
- emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
+ // Fail open to a pull-payment pattern instead of a push transfer, so a single
+ // unreceivable recoveryAddress cannot brick fund recovery permanently.
+ try IERC20(address(stakeToken)).transfer(recoveryAddress, toSweep) returns (bool success) {
+ if (!success) revert TransferFailed();
+ emit ClaimCorrupted(msg.sender, recoveryAddress, toSweep);
+ } catch {
+ pendingRecoverySweep += toSweep;
+ emit ClaimCorruptedQueued(msg.sender, recoveryAddress, toSweep);
+ }

(the queued-sweep pattern requires adding pendingRecoverySweep storage and a permissionless
withdrawQueuedSweep(address to) callable once recoveryAddress is fixed or a grace period
elapses — a minimum viable fix is instead adding a MODERATOR_CORRUPTED_GRACE-style
permissionless fallback recipient, mirroring the pattern this contract already uses for
moderator unavailability, rather than leaving recoveryAddress as a true single point of
failure.)

Support

FAQs

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

Give us feedback!