Root + Impact
Description
The factory's token allowlist stores only an owner-selected boolean. It does not verify that the address implements ERC20, enforce fee-free/non-rebasing behavior, or prevent an initially standard token from changing semantics after pool creation, even though pool accounting assumes those properties for its entire lifetime.
The factory documentation correctly states that confidence pools require standard ERC20 behavior: no transfer fees, rebases, sender-side charges, or mutable policies that can erode custody or block payouts. The allowlist is the only canonical creation gate intended to uphold that operational restriction.
On-chain, however, approval is just a mapping write. createPool() later checks only that boolean, and the clone stores the token address permanently. Delisting does not affect existing pools. An EOA or unrelated contract can therefore be approved by mistake, and an upgradeable/admin-mutable token can behave normally when admitted and during deposits before later introducing fees, rebases, pauses, or blacklists.
src/ConfidencePoolFactory.sol
mapping(address token => bool allowed) public override allowedStakeToken;
function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
}
function setStakeTokenAllowed(address token, bool allowed) external onlyOwner {
if (token == address(0)) revert ZeroAddress();
allowedStakeToken[token] = allowed;
emit StakeTokenAllowedUpdated(token, allowed);
}
The PoC uses a token with zero fee at allowlisting, pool creation, and deposit time. After the pool books a 100-token liability, the token changes to a 10% transfer fee and the factory owner delists it. The existing pool still sends the nominal 100-token claim, clears the claimant's accounting, and reaches a zero balance, while the claimant receives only 90 tokens.
Risk
Likelihood:
-
The condition requires factory-owner vetting failure, owner compromise, or a token with upgradeable/admin-mutable behavior that changes after admission.
-
The factory already documents the standard-token assumption, so careful operational review lowers likelihood, but no contract invariant enforces the review result.
Impact:
-
Mutable fees silently underpay every claimant or recovery recipient while events and pool accounting record nominal payment.
-
Negative rebases, pauses, blacklists, gas bombs, or broken balanceOf/transfer functions can strand later claims or make the entire pool nonfunctional.
Proof of Concept
Create test/audit/CP020TokenAllowlistSemantics.t.sol with the following contents:
pragma solidity 0.8.26;
import {Test} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {ConfidencePoolFactory} from "src/ConfidencePoolFactory.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockFeeOnTransferERC20} from "test/mocks/MockFeeOnTransferERC20.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
contract CP020TokenAllowlistSemanticsTest is Test {
uint256 internal constant ONE = 1e18;
address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);
MockFeeOnTransferERC20 internal mutableToken;
MockAttackRegistry internal attackRegistry;
MockSafeHarborRegistry internal registry;
MockAgreement internal agreement;
ConfidencePoolFactory internal factory;
address internal agreementOwner = makeAddr("agreementOwner");
address internal moderator = makeAddr("moderator");
address internal recovery = makeAddr("recovery");
address internal alice = makeAddr("alice");
function setUp() external {
mutableToken = new MockFeeOnTransferERC20(0);
attackRegistry = new MockAttackRegistry();
registry = new MockSafeHarborRegistry();
agreement = new MockAgreement(agreementOwner);
agreement.setContractInScope(SCOPE_ACCOUNT, true);
registry.setAttackRegistry(address(attackRegistry));
registry.setAgreementValid(address(agreement), true);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
ConfidencePool poolImplementation = new ConfidencePool();
ConfidencePoolFactory factoryImplementation = new ConfidencePoolFactory();
factory = ConfidencePoolFactory(
address(
new ERC1967Proxy(
address(factoryImplementation),
abi.encodeCall(
ConfidencePoolFactory.initialize, (address(registry), address(poolImplementation), moderator)
)
)
)
);
}
function test_AllowlistedStandardTokenCanChangeAndUnderpayExistingPool() external {
factory.setStakeTokenAllowed(address(mutableToken), true);
vm.prank(agreementOwner);
ConfidencePool created = ConfidencePool(
factory.createPool(
address(agreement), address(mutableToken), block.timestamp + 31 days, ONE, recovery, _scope()
)
);
mutableToken.mint(alice, 100 * ONE);
vm.startPrank(alice);
mutableToken.approve(address(created), 100 * ONE);
created.stake(100 * ONE);
vm.stopPrank();
assertEq(created.eligibleStake(alice), 100 * ONE, "token was standard when admitted and deposited");
factory.setStakeTokenAllowed(address(mutableToken), false);
mutableToken.setFeeBps(1_000);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
created.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
vm.prank(alice);
created.claimSurvived();
assertEq(mutableToken.balanceOf(alice), 90 * ONE, "post-allowlist fee silently underpays by ten percent");
assertEq(created.eligibleStake(alice), 0, "pool records the nominal claim as complete");
assertEq(mutableToken.balanceOf(address(created)), 0);
}
function _scope() internal pure returns (address[] memory accounts) {
accounts = new address[](1);
accounts[0] = SCOPE_ACCOUNT;
}
}
Run the PoC from the repository root:
-
Execute forge test --offline --match-path test/audit/CP020TokenAllowlistSemantics.t.sol -vv.
-
Confirm that the test passes: the token is fee-free when allowed and deposited, then the existing pool underpays the fully cleared 100-token claim by 10% after semantics change and delisting.
Recommended Mitigation
At minimum, reject no-code/non-ERC20 addresses and bind approvals to an expected runtime code hash checked again during creation. Treat this only as a sanity check: proxy runtime code hashes do not change when their implementations upgrade, and ERC20 has no generic interface that proves fee-free, non-rebasing semantics.
src/ConfidencePoolFactory.sol
+import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+
+mapping(address token => bytes32 codehash) public allowedStakeTokenCodehash;
+
-uint256[45] private __gap;
+uint256[44] private __gap;
function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
- if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
+ if (
+ !allowedStakeToken[stakeToken]
+ || allowedStakeTokenCodehash[stakeToken] != stakeToken.codehash
+ ) revert StakeTokenNotAllowed();
// ... clone deployment and initialization omitted ...
}
function setStakeTokenAllowed(address token, bool allowed) external onlyOwner {
if (token == address(0)) revert ZeroAddress();
+ if (allowed) {
+ if (token.code.length == 0) revert InvalidStakeToken();
+ (bool ok, bytes memory data) = token.staticcall(
+ abi.encodeCall(IERC20.balanceOf, (address(this)))
+ );
+ if (!ok || data.length != 32) revert InvalidStakeToken();
+ allowedStakeTokenCodehash[token] = token.codehash;
+ } else {
+ delete allowedStakeTokenCodehash[token];
+ }
allowedStakeToken[token] = allowed;
emit StakeTokenAllowedUpdated(token, allowed);
}
Operationally reject proxy/admin-mutable, rebasing, fee-charging, pausable, and blacklist-bearing tokens unless the protocol explicitly supports their behavior through a vetted immutable adapter. Add outbound recipient balance-delta checks so unsupported semantic changes revert visibly instead of silently finalizing underpayment, and monitor/delist affected tokens while recognizing that delisting cannot repair existing pools.
