Unchecked transferFrom Return in fundContest — Contest Appears Funded With Zero Tokens
Description
The ContestManager.fundContest() function calls token.transferFrom(msg.sender, address(pot), totalRewards) without checking the boolean return value. The function only validates that the caller has sufficient balance via token.balanceOf(msg.sender) < totalRewards, but does not verify that the transfer actually succeeded. For non-reverting ERC20 tokens that return false on failure (e.g., insufficient allowance), the contest transitions to a "funded" state in the protocol's view while the Pot contract holds zero tokens.
This creates a particularly dangerous inconsistency: the contestToTotalRewards mapping shows the expected amount, getRemainingRewards() returns the full value, but the actual token balance of the Pot is zero. When players call claimCut(), they will either receive nothing (non-reverting token) or trigger a revert (standard ERC20). In both cases, the contest is rendered non-functional despite appearing properly funded.
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
token.transferFrom(msg.sender, address(pot), totalRewards);
}
Risk
Likelihood
Medium
Impact
High
Severity
Medium
Proof of Concept
The contest owner creates a contest with a non-reverting ERC20 token. They call fundContest() without first calling token.approve(). The balanceOf check passes (owner has tokens), but transferFrom returns false (no allowance). The function completes without error, but the Pot has no tokens. When a player tries claimCut(), the transfer fails silently.
pragma solidity ^0.8.20;
import {Test} from "forge-std/Test.sol";
import {ContestManager} from "../src/ContestManager.sol";
import {Pot} from "../src/Pot.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
contract NonRevertingToken {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
function mint(address to, uint256 amt) external { balanceOf[to] += amt; }
function approve(address spender, uint256 amt) external returns (bool) {
allowance[msg.sender][spender] = amt;
return true;
}
function transfer(address to, uint256 amt) external returns (bool) {
if (balanceOf[msg.sender] < amt) return false;
balanceOf[msg.sender] -= amt;
balanceOf[to] += amt;
return true;
}
function transferFrom(address from, address to, uint256 amt) external returns (bool) {
if (allowance[from][msg.sender] < amt) return false;
balanceOf[from] -= amt;
balanceOf[to] += amt;
allowance[from][msg.sender] -= amt;
return true;
}
}
contract FundContestTest is Test {
function test_fundWithoutApproval_silentFail() public {
NonRevertingToken token = new NonRevertingToken();
ContestManager cm = new ContestManager();
token.mint(address(this), 100 ether);
address[] memory players = new address[](1);
players[0] = address(0xBEEF);
uint256[] memory rewards = new uint256[](1);
rewards[0] = 100 ether;
address potAddr = cm.createContest(players, rewards, IERC20(address(token)), 100 ether);
cm.fundContest(0);
assertEq(token.balanceOf(potAddr), 0);
assertEq(cm.getContestTotalRewards(potAddr), 100 ether);
}
}
Recommended Mitigation
Use SafeERC20.safeTransferFrom() which reverts on false return. Additionally, verify the Pot's token balance increased by the expected amount to defend against fee-on-transfer tokens.
// ContestManager.sol
+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
contract ContestManager is Ownable {
+ using SafeERC20 for IERC20;
// ...
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
- token.transferFrom(msg.sender, address(pot), totalRewards);
+ uint256 balBefore = token.balanceOf(address(pot));
+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);
+ require(token.balanceOf(address(pot)) - balBefore == totalRewards, "transfer mismatch");
}
}
