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

L-01. Balance-diff on inbound transfers only; outbound transfers lose value on fee tokens

Author Revealed upon completion

Root + Impact

stake() and contributeBonus() use balance-diff to measure actual tokens received, correctly handling fee-on-transfer tokens. All outbound transfers (withdraw, claim*, sweep*) use direct safeTransfer(amount) without balance-diff. When the stake token deducts a transfer fee, recipients receive less than entitled. The shortfall accumulates in the contract as unbacked liabilities.

Description

stake() measures actual receipt via balance-diff. If the token takes 1% on a 100-token deposit, the protocol credits 99 tokens:

uint256 balanceBefore = stakeToken.balanceOf(address(this));
stakeToken.safeTransferFrom(msg.sender, address(this), amount);
uint256 balanceAfter = stakeToken.balanceOf(address(this));
uint256 received = balanceAfter > balanceBefore ? balanceAfter - balanceBefore : 0; // @> correct

Every outbound transfer sends the recorded amount without re-measuring:

stakeToken.safeTransfer(msg.sender, amount); // @> line 317 — no balance-diff
stakeToken.safeTransfer(msg.sender, payout); // @> line 403 — no balance-diff

When the token deducts a fee on the outgoing transfer, the user receives less than the protocol recorded as owed. The lost fee stays in the contract and is eventually swept to recoveryAddress — not returned to the staker.

The README states fee-on-transfer tokens are not supported. This finding is about the incomplete defense: the balance-diff on inbound creates a false expectation that the protocol handles such tokens, while outbound paths lack the corresponding protection.

Risk

Likelihood:

  • The factory owner adds a token to allowedStakeToken that charges a transfer fee — either at allowlist time or through a future proxy upgrade. The README disclaims this, but no on-chain check prevents it.

  • A fee-on-transfer token is used as the stake token. Every outbound transfer deducts a fee the protocol does not account for.

Impact:

  • Each staker receives less than their recorded eligibleStake or payout on every withdraw or claim. The shortfall accumulates in the contract and is swept to recoveryAddress, not returned to the staker.

  • The accounting variables (totalEligibleStake, claimedBonus) diverge from the actual token balance, making true solvency unverifiable through accounting alone.

Proof of Concept

The test deploys a pool with MockFeeOnTransferERC20 (1% fee on every transfer). Alice deposits 1000 tokens and immediately withdraws.

Timeline and math:

  1. Alice deposits 1000 tokens into the pool.

  2. The 1% fee deducts 10 tokens on the inbound transfer. The protocol's balance-diff correctly credits Alice with 990 tokens (eligibleStake[alice] == 990).

  3. Alice calls withdraw(). The protocol calls stakeToken.safeTransfer(alice, 990).

  4. The 1% fee deducts ~9.9 tokens on the outbound transfer. Alice receives ~980 tokens, not the 990 the protocol recorded.

  5. Alice lost 1% on entry (990 credited instead of 1000) and 1% on exit (980 received instead of 990). The 9.9-token fee stays in the pool contract permanently — it can only be swept to recoveryAddress, not returned to Alice.

Test output:

[PASS] test_L1_withdrawLosesOnePercent() (gas: 292795)
[PASS] test_L1_withdrawCreditedAmountLessThanDeposited() (gas: 292342)

Both tests confirm: the protocol correctly credits 990 tokens on deposit, but Alice receives only ~980 on withdrawal — a double loss from the asymmetric fee handling.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {MockFeeOnTransferERC20} from "test/mocks/MockFeeOnTransferERC20.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract TestL1_BalanceDiffAsymmetry is BaseConfidencePoolTest {
MockFeeOnTransferERC20 internal feeToken;
ConfidencePool internal feePool;
​
function setUp() public override {
super.setUp();
feeToken = new MockFeeOnTransferERC20(100); // 100 bps = 1%
feePool = _deployFeePool(address(feeToken));
}
​
function test_L1_withdrawLosesOnePercent() external {
uint256 depositAmount = 1000 * ONE;
feeToken.mint(alice, depositAmount);
​
vm.startPrank(alice);
feeToken.approve(address(feePool), depositAmount);
feePool.stake(depositAmount);
vm.stopPrank();
​
uint256 credited = feePool.eligibleStake(alice);
assertEq(credited, depositAmount * 990 / 1000); // 990 tokens
​
uint256 before = feeToken.balanceOf(alice);
vm.prank(alice);
feePool.withdraw();
uint256 received = feeToken.balanceOf(alice) - before;
​
// Alice receives 990 - 1% = ~980
assertEq(received, (credited * 990) / 1000, "Fee deducted on both entry and exit");
}
​
function test_L1_withdrawCreditedAmountLessThanDeposited() external {
uint256 depositAmount = 1000 * ONE;
feeToken.mint(alice, depositAmount);
​
vm.startPrank(alice);
feeToken.approve(address(feePool), depositAmount);
feePool.stake(depositAmount);
vm.stopPrank();
​
uint256 credited = feePool.eligibleStake(alice);
assertEq(credited, depositAmount * 990 / 1000, "Entry fee: credited 990 of 1000");
​
uint256 before = feeToken.balanceOf(alice);
vm.prank(alice);
feePool.withdraw();
uint256 received = feeToken.balanceOf(alice) - before;
​
assertEq(received, (credited * 990) / 1000, "Exit fee: received less than credited");
}
​
function _deployFeePool(address feeToken_) internal returns (ConfidencePool) {
ConfidencePool impl = new ConfidencePool();
ConfidencePool p = ConfidencePool(Clones.clone(address(impl)));
address[] memory scope = _defaultScope();
p.initialize(
agreement, feeToken_, address(safeHarborRegistry),
moderator, block.timestamp + 31 days, ONE,
recovery, address(this), scope
);
return p;
}
}

Recommended Mitigation

Apply the same balance-diff pattern to outbound transfers. For withdraw():

function withdraw() external nonReentrant {
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
// ... existing accumulator cleanup ...
- stakeToken.safeTransfer(msg.sender, amount);
+ uint256 balanceBefore = stakeToken.balanceOf(address(this));
+ stakeToken.safeTransfer(msg.sender, amount);
+ uint256 balanceAfter = stakeToken.balanceOf(address(this));
+ uint256 transferred = balanceBefore - balanceAfter;
+ if (transferred < amount) {
+ totalEligibleStake -= amount - transferred;
+ }
emit Withdrawn(msg.sender, transferred);
}

Apply the same pattern to claimSurvived() and claimExpired() around the final safeTransfer. The sweepUnclaimedBonus() function already reads balanceOf(this) before the sweep and caps at freeBalance - reserved, so it naturally handles fee tokens.

Support

FAQs

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

Give us feedback!