Root + Impact
Description
Every outbound pool path assumes that transferring a nominal ERC20 amount increases the recipient's balance by the same amount. A recipient-side fee can instead burn or divert part of the transfer while SafeERC20.safeTransfer() still succeeds.
The pool updates liabilities, claim flags, bounty progress, and reserves using the nominal amount. It does not measure recipient delivery afterward. Withdrawers, SURVIVED/EXPIRED claimants, named attackers, and recovery recipients can therefore receive less than the amount that events and accounting mark as completely paid.
Inbound stake and bonus calls measure the pool's balance change, but outbound calls use an unchecked nominal transfer. The pattern appears throughout the contract:
src/ConfidencePool.sol
function withdraw() external nonReentrant {
uint256 amount = eligibleStake[msg.sender];
totalEligibleStake -= amount;
stakeToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
}
function claimSurvived() external nonReentrant {
uint256 payout = userEligible + bonusShare;
stakeToken.safeTransfer(msg.sender, payout);
emit ClaimSurvived(msg.sender, userEligible, bonusShare);
}
function claimAttackerBounty() external nonReentrant {
uint256 payout = remaining <= freeBalance ? remaining : freeBalance;
bountyClaimed += payout;
corruptedReserve -= payout;
stakeToken.safeTransfer(attacker, payout);
}
function claimCorrupted() external nonReentrant {
uint256 toSweep = stakeToken.balanceOf(address(this));
stakeToken.safeTransfer(recoveryAddress, toSweep);
}
The same issue affects claimExpired(), sweepUnclaimedCorrupted(), and sweepUnclaimedBonus(). With an ordinary recipient-fee token, the pool's balance falls by the gross amount and its nominal accounting stays internally solvent, so no later invariant exposes the shortfall. The fee loss is borne silently by each recipient.
The PoC token is fee-free during deposit, allowing a 100-token liability to be recorded exactly. It then enables a 10% fee before each exit. In every test the pool balance drops from 100 to zero and its accounting marks 100 as paid or swept, while the intended recipient receives only 90.
Fee-on-transfer assets are explicitly unsupported by the factory's documented token policy. The condition therefore requires allowlist error or a token whose admin/upgrade path changes semantics after pool creation. The pool's inbound balance-difference logic and mutable allowlist do not protect already-created pools from that change.
Risk
Likelihood:
-
An immutable standard ERC20 cannot trigger the issue; an admitted fee token or post-admission semantic change is required.
-
Governance token review lowers likelihood, but existing pools retain their token even after factory delisting.
-
Once a recipient-side fee is enabled, every affected outbound call deterministically under-delivers.
Impact:
-
Users silently lose part of withdrawn principal and claimable principal/bonus while their account is fully cleared.
-
A good-faith attacker can be marked as having claimed the full bounty despite receiving less than bountyEntitlement.
-
Recovery sweeps can empty the pool and zero reserve bookkeeping while the recovery recipient receives less than the recorded amount.
Proof of Concept
Create test/audit/CP016FeeOnTransferOutboundUnderpayment.t.sol with the following contents:
pragma solidity 0.8.26;
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";
import {MockFeeOnTransferERC20} from "test/mocks/MockFeeOnTransferERC20.sol";
contract CP016FeeOnTransferOutboundUnderpaymentTest is BaseConfidencePoolTest {
MockFeeOnTransferERC20 internal feeToken;
ConfidencePool internal feePool;
function setUp() public override {
super.setUp();
feeToken = new MockFeeOnTransferERC20(0);
feePool = _deployPoolWithToken(address(feeToken));
}
function test_WithdrawClearsNominalStakeButDeliversNinetyPercent() external {
_stakeFeeToken(alice, 100 * ONE);
feeToken.setFeeBps(1_000);
uint256 aliceBefore = feeToken.balanceOf(alice);
vm.prank(alice);
feePool.withdraw();
assertEq(feeToken.balanceOf(alice) - aliceBefore, 90 * ONE);
assertEq(feePool.eligibleStake(alice), 0, "the nominal 100-token stake is fully cleared");
assertEq(feePool.totalEligibleStake(), 0);
assertEq(feeToken.balanceOf(address(feePool)), 0);
}
function test_SurvivedClaimRecordsFullPaymentButDeliversNinetyPercent() external {
_stakeFeeToken(alice, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
feePool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
feeToken.setFeeBps(1_000);
uint256 aliceBefore = feeToken.balanceOf(alice);
vm.prank(alice);
feePool.claimSurvived();
assertEq(feeToken.balanceOf(alice) - aliceBefore, 90 * ONE);
assertTrue(feePool.hasClaimed(alice));
assertEq(feePool.eligibleStake(alice), 0, "claim accounting records the full nominal payout");
}
function test_AttackerBountyRecordsFullEntitlementButDeliversNinetyPercent() external {
_stakeFeeToken(alice, 100 * ONE);
_flagCorrupted(true, attacker);
feeToken.setFeeBps(1_000);
uint256 attackerBefore = feeToken.balanceOf(attacker);
vm.prank(attacker);
feePool.claimAttackerBounty();
assertEq(feeToken.balanceOf(attacker) - attackerBefore, 90 * ONE);
assertEq(feePool.bountyEntitlement(), 100 * ONE);
assertEq(feePool.bountyClaimed(), 100 * ONE, "bookkeeping marks the bounty fully delivered");
assertEq(feeToken.balanceOf(address(feePool)), 0);
}
function test_CorruptedSweepEmptiesPoolButDeliversNinetyPercentToRecovery() external {
_stakeFeeToken(alice, 100 * ONE);
_flagCorrupted(false, address(0));
feeToken.setFeeBps(1_000);
uint256 recoveryBefore = feeToken.balanceOf(recovery);
feePool.claimCorrupted();
assertEq(feeToken.balanceOf(recovery) - recoveryBefore, 90 * ONE);
assertEq(feeToken.balanceOf(address(feePool)), 0, "the pool debits the full nominal sweep");
assertEq(feePool.corruptedReserve(), 0, "reserve accounting also records full completion");
}
function _flagCorrupted(bool goodFaith, address namedAttacker) internal {
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
feePool.pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
feePool.flagOutcome(PoolStates.Outcome.CORRUPTED, goodFaith, namedAttacker);
}
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()
);
}
function _stakeFeeToken(address user, uint256 amount) internal {
feeToken.mint(user, amount);
vm.startPrank(user);
feeToken.approve(address(feePool), amount);
feePool.stake(amount);
vm.stopPrank();
}
}
Run the PoC from the repository root:
-
Execute forge test --offline --match-path test/audit/CP016FeeOnTransferOutboundUnderpayment.t.sol -vv.
-
Confirm that all four tests pass. Each path records or clears a nominal 100-token obligation and empties the corresponding pool balance, but the withdrawer, claimant, attacker, or recovery recipient receives only 90 tokens.
Recommended Mitigation
Centralize outbound transfers in a helper that checks the recipient's balance delta and reverts unless the exact nominal amount arrived. Because a revert is atomic, the preceding liability and claim-state changes also roll back instead of recording false completion.
src/ConfidencePool.sol
+function _safeTransferExact(address recipient, uint256 amount) internal {
+ uint256 beforeBalance = stakeToken.balanceOf(recipient);
+ stakeToken.safeTransfer(recipient, amount);
+ uint256 afterBalance = stakeToken.balanceOf(recipient);
+ if (afterBalance < beforeBalance || afterBalance - beforeBalance != amount) {
+ revert UnsupportedTokenSemantics();
+ }
+}
function withdraw() external nonReentrant {
// ...
- stakeToken.safeTransfer(msg.sender, amount);
+ _safeTransferExact(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
}
function claimAttackerBounty() external nonReentrant {
// ...
- stakeToken.safeTransfer(attacker, payout);
+ _safeTransferExact(attacker, payout);
}
function claimCorrupted() external nonReentrant {
// ...
- stakeToken.safeTransfer(recoveryAddress, toSweep);
+ _safeTransferExact(recoveryAddress, toSweep);
}
Use the helper for every withdrawal, claim, bounty, and sweep. This converts silent loss into a visible unsupported-token failure; it does not make mutable fee tokens safe, because claims will remain blocked until the token restores standard behavior. Prevent that state operationally by admitting only immutable, fee-free tokens or immutable vetted adapters and by treating upgradeable/admin-mutable tokens as unsupported. If fee-on-transfer support is an explicit goal, define payouts in net-received units or a token-specific gross-up adapter rather than attempting generic ERC20 exact-delivery accounting.
