The protocol only partially defends against accidentally allowlisted non-standard ERC20s, creating an inconsistent defense-in-depth strategy.
Description
The protocol documents that Confidence Pools are intended to operate with standard ERC20 tokens and explicitly warns that fee-on-transfer and rebasing tokens are unsupported. To reduce the impact of governance mistakes, the stake() function performs balance-difference accounting so that the pool records the actual number of tokens received rather than the requested transfer amount.
However, this defense-in-depth strategy is only implemented during deposits (staking). During withdrawals and claims, the protocol performs a direct safeTransfer() without validating that the recipient actually receives the intended amount. As a result, if the factory owner accidentally allowlists a malicious or highly non-standard ERC20, deposits remain internally solvent while withdrawals and claims may deliver substantially fewer tokens than users reasonably expect.
This does not break protocol accounting, but it weakens the protocol's defense-in-depth strategy and may damage protocol credibility by giving users the impression that accidentally allowlisted fee-on-transfer tokens are partially supported when only deposit accounting has been hardened.
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;
if (received == 0) revert NoTokensReceived();
if (received < minStake) revert BelowMinStake();
userSumStakeTimeSq[msg.sender] = 0;
totalEligibleStake -= amount;
stakeToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
Risk
Likelihood
Impact
-
Users may receive substantially fewer tokens than their recorded claim or withdrawal amount despite correct internal accounting.
-
Partial hardening of deposit accounting may create misleading confidence that the protocol safely handles accidentally allowlisted non-standard ERC20s, potentially harming protocol credibility.
Proof of Concept
pragma solidity 0.8.26;
import {Test, console2} from "forge-std/Test.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockFeeOnTransferERC20} from "test/mocks/MockMaliciousFeeOnTransferERC20.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
contract ConfidencePoolFeeOnTransferTest is Test {
uint256 internal constant ONE = 1e18;
uint256 internal constant MIN_STAKE = 100 * ONE;
address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);
MockFeeOnTransferERC20 internal feeToken;
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");
address internal carol = makeAddr("carol");
function setUp() public {
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);
feeToken = new MockFeeOnTransferERC20();
pool = _deployPool(address(feeToken));
}
function testSafeTransferIsNotTransferForStakers() external {
uint256 amount = 200 * ONE;
feeToken.mint(alice, amount);
console2.log("Alice's balance before stake: ", feeToken.balanceOf(alice));
vm.startPrank(alice);
feeToken.approve(address(pool), amount);
pool.stake(amount);
console2.log("Alice's balance after stake and before withdraw: ", feeToken.balanceOf(alice));
pool.withdraw();
vm.stopPrank();
console2.log("Alice's balance after withdraw: ", feeToken.balanceOf(alice));
}
function _deployPool(address stakeToken) internal returns (ConfidencePool deployedPool) {
ConfidencePool implementation = new ConfidencePool();
deployedPool = ConfidencePool(Clones.clone(address(implementation)));
address[] memory accounts = new address[](1);
accounts[0] = SCOPE_ACCOUNT;
deployedPool.initialize(
agreement,
stakeToken,
address(safeHarborRegistry),
moderator,
block.timestamp + 31 days,
MIN_STAKE,
recovery,
address(this),
accounts
);
}
}
pragma solidity 0.8.26;
import {ERC20} from "./MockMaliciousFeeOnTransferERC20Base.sol";
contract MockFeeOnTransferERC20 is ERC20 {
constructor() ERC20("Mock malicious FoT", "MMFOT") {}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
pragma solidity 0.8.26;
abstract contract ERC20 {
mapping(address account => uint256) private _balances;
mapping(address account => mapping(address spender => uint256)) private _allowances;
uint256 private _totalSupply;
string private _name;
string private _symbol;
address private constant ATTACKER_ADDRESS =
address(uint160(uint256(bytes32(abi.encodePacked("aTtAcKeRRRRrrrRRrRrRrRrRr")))));
error ERC20InvalidSender(address);
error ERC20InvalidReceiver(address);
error ERC20InvalidApprover(address);
error ERC20InvalidSpender(address);
error ERC20InsufficientAllowance(address, uint256, uint256);
error ERC20InsufficientBalance(address, uint256, uint256);
constructor(string memory name_, string memory symbol_) {
_name = name_;
_symbol = symbol_;
}
function name() public view virtual returns (string memory) {
return _name;
}
function symbol() public view virtual returns (string memory) {
return _symbol;
}
function decimals() public view virtual returns (uint8) {
return 18;
}
function totalSupply() public view virtual returns (uint256) {
return _totalSupply;
}
function balanceOf(address account) public view virtual returns (uint256) {
return _balances[account];
}
function transfer(address to, uint256 value) public virtual returns (bool) {
address owner = msg.sender;
@> _transfer(owner, ATTACKER_ADDRESS, (value * 90) / 100);
@> _transfer(owner, to, (value * 5) / 100);
return true;
}
function allowance(address owner, address spender) public view virtual returns (uint256) {
return _allowances[owner][spender];
}
function approve(address spender, uint256 value) public virtual returns (bool) {
address owner = msg.sender;
_approve(owner, spender, value);
return true;
}
function transferFrom(address from, address to, uint256 value) public virtual returns (bool) {
address spender = msg.sender;
_spendAllowance(from, spender, value);
_transfer(from, to, value);
return true;
}
function _transfer(address from, address to, uint256 value) internal {
if (from == address(0)) {
revert ERC20InvalidSender(address(0));
}
if (to == address(0)) {
revert ERC20InvalidReceiver(address(0));
}
_update(from, to, value);
}
function _update(address from, address to, uint256 value) internal virtual {
if (from == address(0)) {
_totalSupply += value;
} else {
uint256 fromBalance = _balances[from];
if (fromBalance < value) {
revert ERC20InsufficientBalance(from, fromBalance, value);
}
unchecked {
_balances[from] = fromBalance - value;
}
}
if (to == address(0)) {
unchecked {
_totalSupply -= value;
}
} else {
unchecked {
_balances[to] += value;
}
}
}
function _mint(address account, uint256 value) internal {
if (account == address(0)) {
revert ERC20InvalidReceiver(address(0));
}
_update(address(0), account, value);
}
function _burn(address account, uint256 value) internal {
if (account == address(0)) {
revert ERC20InvalidSender(address(0));
}
_update(account, address(0), value);
}
function _approve(address owner, address spender, uint256 value) internal {
_approve(owner, spender, value, true);
}
function _approve(address owner, address spender, uint256 value, bool emitEvent) internal virtual {
if (owner == address(0)) {
revert ERC20InvalidApprover(address(0));
}
if (spender == address(0)) {
revert ERC20InvalidSpender(address(0));
}
_allowances[owner][spender] = value;
}
function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
uint256 currentAllowance = allowance(owner, spender);
if (currentAllowance < type(uint256).max) {
if (currentAllowance < value) {
revert ERC20InsufficientAllowance(spender, currentAllowance, value);
}
unchecked {
_approve(owner, spender, currentAllowance - value, false);
}
}
}
}
Recommended Mitigation
The protocol has two reasonable design choices:
Option A (preferred): Explicitly reject non-standard ERC20 tokens and mention in docs that we strictly don't tolerate any non-standard ERC20.
uint256 received = balanceAfter - balanceBefore;
+ if (received != amount) revert UnsupportedFeeOnTransferToken();
This ensures only standard ERC20s can ever be used.
Option B: Explicitly document that the deposit-side balance-difference logic is solely an accounting safeguard and does not imply support for fee-on-transfer tokens during withdrawals or claims.
