MyCut

AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Submission Details
Impact: high
Likelihood: medium
Invalid

Incorrect Balance Check in fundContest()

Root + Impact

Description

  • The `fundContest()` function checks if the owner has sufficient token balance, but it should check the allowance instead. The function uses `transferFrom()`, which requires an allowance, not just a balance. This check can incorrectly revert when the owner has sufficient balance but insufficient allowance, or incorrectly proceed when the owner has sufficient allowance but the check passes due to balance.

    The function checks `token.balanceOf(msg.sender)` but `msg.sender` is the owner (due to `onlyOwner`), and the actual transfer happens from `msg.sender` to the Pot. However, if tokens were approved from a different address, this check is meaningless.

```solidity
// Root cause in the codebase
33| if (token.balanceOf(msg.sender) < totalRewards) {
34| revert ContestManager__InsufficientFunds();
35| }
36|
37| token.transferFrom(msg.sender, address(pot), totalRewards);
```

Risk

Likelihood:

  • * This occurs when the owner attempts to fund a contest

    * The issue manifests when tokens are approved from a different address than the owner, or when the owner has balance but hasn't approved the ContestManager

Impact:

  • * Function may revert incorrectly when owner has balance but insufficient allowance

    * Function may proceed incorrectly if the check passes but `transferFrom()` fails due to insufficient allowance

    * The error message "InsufficientFunds" is misleading when the actual issue is insufficient allowance

    * Poor user experience due to incorrect error handling

Proof of Concept

```solidity
// Scenario 1: Owner has balance but no allowance
// - Owner balance: 1000 tokens
// - Owner allowance to ContestManager: 0 tokens
// - totalRewards: 500 tokens
// - Check passes (balance >= 500)
// - transferFrom() fails due to insufficient allowance
// - Transaction reverts with unclear error
// Scenario 2: Tokens approved from different address
// - Owner balance: 0 tokens
// - Another address approved 1000 tokens to ContestManager
// - Check fails (owner balance < 500)
// - Function reverts even though transferFrom() would succeed
```

Recommended Mitigation

```diff
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) {
+ if (token.allowance(msg.sender, address(this)) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
token.transferFrom(msg.sender, address(pot), totalRewards);
}
```
Alternatively, check both balance and allowance:
```diff
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();
- }
+ if (token.balanceOf(msg.sender) < totalRewards) {
+ revert ContestManager__InsufficientFunds();
+ }
+ if (token.allowance(msg.sender, address(this)) < totalRewards) {
+ revert ContestManager__InsufficientAllowance();
+ }
token.transferFrom(msg.sender, address(pot), totalRewards);
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!