BattleChain Confidence Pools

Cyfrin

FoundrySolidityLayer 2

7.25 ETH

View all submissions

Previous Next

Submission Details

Impact: medium

Likelihood: medium

Sender-fee tokens can freeze settlement despite full nominal reserves

Author Revealed upon completion

Description

ConfidencePool assumes its token balance is fully transferable to claimants and sweep recipients. The root cause is that outbound settlement sends the nominal payout amount without accounting for sender-side transfer costs, so a fee-on-sender token can make claims or CORRUPTED sweeps revert despite apparently full reserves.

Details

The repo does say only standard ERC20s are supported, and the factory comment acknowledges that fee-on-sender or negative-rebasing tokens can erode the pool below liabilities. But the broader protocol docs make a stronger claim: the protocol readme says the owner cannot freeze settlements because claim and sweep paths stay callable, and the design doc says a true stuck state would require a token that freezes the pool's own balance. Sender-fee semantics are materially worse than that description: the pool balance can remain healthy while outbound settlement is still impossible.

The survivor and expiry claim paths never check whether the requested payout is actually transferable under the token's sender-side semantics. claimSurvived() computes payout and immediately calls safeTransfer(msg.sender, payout), and claimExpired() does the same after auto-resolution.

uint256 bonusShare = _bonusShare(msg.sender, userEligible);

uint256 payout = userEligible + bonusShare;

totalEligibleStake -= userEligible;

claimedBonus += bonusShare;

delete eligibleStake[msg.sender];

delete userSumStakeTime[msg.sender];

delete userSumStakeTimeSq[msg.sender];

if (!claimsStarted) claimsStarted = true;

stakeToken.safeTransfer(msg.sender, payout);

For a sender-fee token, transferring payout requires the pool to fund payout + fee. A pool that nominally holds exactly payout therefore becomes unclaimable even though its accounting still says the user is fully covered. With one staker owed 100 and a 10% sender fee, the pool balance is 100, but claimExpired() still reverts because the token burns 10 from the pool before attempting the 100 transfer.

The bad-faith CORRUPTED path makes the same assumption in a stronger form. claimCorrupted() reads the pool's entire balanceOf(address(this)) and tries to sweep that exact amount to recoveryAddress.

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);

That is impossible under sender-fee semantics whenever toSweep is the full pool balance, because paying toSweep requires more than toSweep to leave the sender. The result is a permanently frozen bad-faith CORRUPTED reserve even though the contract still holds the entire nominal balance.

The same root cause also creates claimant-ordering insolvency. If Alice and Bob each have 100 principal and the token burns 10% from the sender, Alice can claim first and receive her full 100, but the pool spends 110 to do it. Bob is still owed 100, yet only 90 remains, so his later claim reverts. This is stronger than the repo's documented "later claims may lock" warning: one rightful claimant can consume another claimant's reachable reserve simply by claiming first. The current test suite does not cover that surface. test/unit/ConfidencePool.fot.t.sol only validates inbound balance-diff accounting for stake() and contributeBonus(); it never exercises outbound claim or sweep settlement under non-standard sender-side fees.

Risk

If governance mistakenly allowlists a sender-fee token, or a previously allowed token upgrades into sender-fee behavior, the pool can become insolvent at settlement time even while totalEligibleStake, corruptedReserve, and the live token balance still look healthy. A lone claimant can be unable to withdraw principal, an early claimant can strand later claimants by burning the common reserve first, and bad-faith CORRUPTED recovery can be frozen permanently. The result is real fund loss by denial of payout, not just benign underpayment.

Recommended mitigation steps

Reject any stake token whose outbound transfers are not exact-value sender-neutral ERC20 transfers, and never assume balanceOf(address(this)) is the maximum transferable amount for claim or sweep settlement.

Proof of concept

I validated this locally with a minimal Foundry regression test.

Save the following as test/TempSenderFeeSemantics.t.sol:

// 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 {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

import {ConfidencePool} from "src/ConfidencePool.sol";

import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";

import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";

import {MockAgreement} from "test/mocks/MockAgreement.sol";

import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";

import {PoolStates} from "src/libraries/PoolStates.sol";

contract TempSenderFeeSemanticsTest is Test {

uint256 internal constant ONE = 1e18;

address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);

PoolSenderFeeERC20 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 carol = makeAddr("carol");

function setUp() public {

token = new PoolSenderFeeERC20(1_000);

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 accounts = new address[](1);

accounts[0] = SCOPE_ACCOUNT;

pool.initialize(

agreement,

address(token),

address(safeHarborRegistry),

moderator,

block.timestamp + 31 days,

ONE,

recovery,

address(this),

accounts

);

token.setFeeSender(address(pool));

}

function testSingleClaimantCanBeLockedOutDespiteFullNominalBalance() external {

_stake(alice, 100 * ONE);

vm.warp(block.timestamp + 32 days);

vm.prank(alice);

vm.expectRevert();

pool.claimExpired();

assertEq(token.balanceOf(address(pool)), 100 * ONE, "pool still holds nominal payout");

assertEq(pool.eligibleStake(alice), 100 * ONE, "principal remains owed");

assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.UNRESOLVED), "resolution rolled back");

}

function testBadFaithCorruptedFullBalanceSweepCanBecomeImpossible() external {

_stake(alice, 100 * ONE);

_contributeBonus(carol, 50 * ONE);

attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);

pool.pokeRiskWindow();

attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);

vm.prank(moderator);

pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));

vm.expectRevert();

pool.claimCorrupted();

assertEq(token.balanceOf(address(pool)), 150 * ONE, "full pool remains trapped");

assertEq(pool.corruptedReserve(), 150 * ONE, "reserve still owed to recovery");

assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED), "pool stays corrupted");

}

function testEarlyClaimantCanConsumeSharedReserveAndStrandLaterClaimant() external {

address bob = makeAddr("bob");

_stake(alice, 100 * ONE);

_stake(bob, 100 * ONE);

vm.warp(block.timestamp + 32 days);

uint256 aliceBefore = token.balanceOf(alice);

vm.prank(alice);

pool.claimExpired();

assertEq(token.balanceOf(alice) - aliceBefore, 100 * ONE, "alice received her full principal");

assertEq(token.balanceOf(address(pool)), 90 * ONE, "sender fee burned shared pool balance");

vm.prank(bob);

vm.expectRevert();

pool.claimExpired();

assertEq(pool.eligibleStake(bob), 100 * ONE, "bob is still owed his principal");

assertEq(token.balanceOf(address(pool)), 90 * ONE, "remaining balance is no longer enough to pay bob");

}

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();

}

contract PoolSenderFeeERC20 is ERC20 {

uint256 public immutable feeBps;

address public feeSender;

constructor(uint256 feeBps_) ERC20("Pool Sender Fee", "PSF") {

feeBps = feeBps_;

}

function setFeeSender(address sender) external {

feeSender = sender;

}

function mint(address to, uint256 amount) external {

_mint(to, amount);

}

function _update(address from, address to, uint256 value) internal override {

if (from == address(0) || to == address(0) || from != feeSender || feeBps == 0) {

super._update(from, to, value);

return;

}

uint256 fee = (value * feeBps) / 10_000;

super._update(from, address(0), fee);

super._update(from, to, value);

}

Run:

forge test --match-path test/TempSenderFeeSemantics.t.sol -vv

Observed output:

Compiling 1 files with Solc 0.8.26

Solc 0.8.26 finished in 5.91s

Compiler run successful!

Ran 3 tests for test/TempSenderFeeSemantics.t.sol:TempSenderFeeSemanticsTest

[PASS] testBadFaithCorruptedFullBalanceSweepCanBecomeImpossible() (gas: 539780)

[PASS] testEarlyClaimantCanConsumeSharedReserveAndStrandLaterClaimant() (gas: 523550)

[PASS] testSingleClaimantCanBeLockedOutDespiteFullNominalBalance() (gas: 426189)

Suite result: ok. 3 passed; 0 failed; 0 skipped; finished in 18.20ms (6.63ms CPU time)

Ran 1 test suite in 31.35ms (18.20ms CPU time): 3 tests passed, 0 failed, 0 skipped (3 total tests)

This PoC shows the three concrete failure modes:

A lone staker can become completely unclaimable even though the pool still holds their full nominal balance.
A bad-faith CORRUPTED pool can become unsweepable even though corruptedReserve equals the full live balance.
With multiple stakers, the first claimant can burn sender fees out of the shared reserve and leave the next claimant permanently short.

Prize pool breakdown

Total prize

7.25 ETH

nSLOC

589

0 ETH / LOC

High Medium

Low

1.25

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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

Give us feedback!