Description
The Pot contract interacts with ERC20 tokens through the IERC20 interface and calls transfer() in multiple locations without checking the returned boolean. The ERC20 standard specifies that transfer() returns false on failure rather than reverting. Non-compliant tokens (such as USDT and other legacy tokens) follow this pattern, returning false when a transfer fails.
In claimCut(), the state is updated (reward zeroed, remainingRewards decremented, player added to claimants) before the transfer executes. When the transfer fails silently, the player's reward is marked as claimed but never delivered, permanently locking their funds.
function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
@> playersToRewards[player] = 0;
@> remainingRewards -= reward;
@> claimants.push(player);
@> _transferReward(player, reward);
}
function _transferReward(address player, uint256 reward) internal {
@> i_token.transfer(player, reward);
}
The same pattern exists in closePot() at lines 55 and 59, where both the manager cut and claimant redistribution transfers ignore the return value.
Risk
Likelihood:
-
A Pot is deployed with a non-standard ERC20 token (USDT on mainnet, various DeFi protocol tokens) that returns false on failed transfers instead of reverting
-
The Pot contract receives a lower-than-expected token balance (e.g., due to fee-on-transfer behavior, partially filled fundContest(), or reentrancy), causing transfers to fail
Impact:
-
Player's reward is marked as claimed and remainingRewards is decremented, but the tokens are never transferred. The player permanently loses their reward with no way to re-claim or recover it.
-
In closePot(), failed manager-cut or claimant-cut transfers lock funds permanently — the Pot retains tokens but remainingRewards is not updated to reflect the failure.
Proof of Concept
pragma solidity ^0.8.20;
import {Pot} from "../../src/Pot.sol";
import {Test} from "lib/forge-std/src/Test.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {FalseReturnERC20} from "./FalseReturnERC20.sol";
contract PocUncheckedTransfer is Test {
FalseReturnERC20 public token;
address public owner = makeAddr("owner");
event Result(string message, uint256 value);
function setUp() public {
vm.startPrank(owner);
token = new FalseReturnERC20("BAD", "BAD");
vm.stopPrank();
}
function test_PoC_ClaimCutSilentFailure() public {
address[] memory players = new address[](2);
players[0] = makeAddr("alice");
players[1] = makeAddr("bob");
uint256[] memory rewards = new uint256[](2);
rewards[0] = 1 ether;
rewards[1] = 1 ether;
vm.startPrank(owner);
token.mint(owner, 2 ether);
Pot pot = new Pot(players, rewards, IERC20(address(token)), 2 ether);
token.transfer(address(pot), 1 ether);
vm.stopPrank();
emit Result("Pot balance:", token.balanceOf(address(pot)));
vm.prank(makeAddr("alice"));
pot.claimCut();
emit Result("After Alice claims - Pot balance:", token.balanceOf(address(pot)));
emit Result("After Alice claims - remainingRewards:", pot.getRemainingRewards());
vm.prank(makeAddr("bob"));
pot.claimCut();
emit Result("After Bob claims - Bob balance:", token.balanceOf(makeAddr("bob")));
emit Result("After Bob claims - remainingRewards:", pot.getRemainingRewards());
assertEq(token.balanceOf(makeAddr("bob")), 0, "Bob got nothing - transfer returned false");
assertEq(pot.getRemainingRewards(), 0, "remainingRewards decremented despite failed transfer");
}
}
pragma solidity 0.8.20;
contract FalseReturnERC20 {
string public name;
string public symbol;
uint8 public decimals = 18;
uint256 public totalSupply;
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
constructor(string memory _name, string memory _symbol) {
name = _name;
symbol = _symbol;
}
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
totalSupply += amount;
}
function approve(address spender, uint256 amount) external returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
function transfer(address to, uint256 amount) external returns (bool) {
if (balanceOf[msg.sender] < amount) {
return false;
}
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
if (balanceOf[from] < amount) {
return false;
}
if (allowance[from][msg.sender] < amount) {
return false;
}
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
}
Recommended Mitigation
function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
- playersToRewards[player] = 0;
- remainingRewards -= reward;
- claimants.push(player);
- _transferReward(player, reward);
+ _transferReward(player, reward);
+ playersToRewards[player] = 0;
+ remainingRewards -= reward;
+ claimants.push(player);
}
function _transferReward(address player, uint256 reward) internal {
- i_token.transfer(player, reward);
+ require(i_token.transfer(player, reward), "Pot: transfer failed");
}
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
- i_token.transfer(msg.sender, managerCut);
+ require(i_token.transfer(msg.sender, managerCut), "Pot: manager transfer failed");
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
