MyCut

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

Missing allowance check in `fundContest()` before `transferFrom`, and unfunded Pots can never be closed

Root + Impact

Two low-risk issues in the funding and closing flow: fundContest() doesn't check token allowance before transferFrom, and a Pot that's never funded can never be closed.

Description

Finding 1:

  • fundContest() checks the Owner's token balance but never checks allowance(), if the Owner hasn't approved ContestManager to spend their tokens, transferFrom reverts with a generic error instead of a clear one.

Finding 2:

  • closePot() assumes a Pot has been funded. If a Pot is created but fundContest() is never successfully called, closePot() will always revert (Pot balance is 0), leaving it permanently stuck open with no way to cancel or recover it.

// Finding 1 — ContestManager.sol
token.transferFrom(msg.sender, address(pot), totalRewards); @> // reverts with unclear error if allowance was never set
// Finding 2 — Pot.sol
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut); @> // reverts — Pot has 0 balance if never funded
...
}

Risk

Likelihood:

  • Occurs whenever the Owner calls fundContest() without first approving the ContestManager contract, an easy step to forget.

  • Occurs whenever a Pot is created but never successfully funded, whether by oversight or a failed funding attempt.

Impact:

  • No funds are lost in either case, both are safe, recoverable states (retry after approving; or the Pot stays open but non-functional).

  • The main cost is reduced usability, an unclear revert message in Finding 1, and no way to cancel/reset a Pot in Finding 2.

Proof of Concept

No PoC is required for either finding, both describe standard, well-understood ERC20/EVM revert behavior (missing allowance, and calling transfer() on a zero balance) rather than an exploitable logic bug. The reverts themselves are self-evident from the code and don't require a test to demonstrate.

Recommended Mitigation

Add an explicit check in closePot() (or a separate cancelPot() function) confirming the Pot has actually been funded before proceeding, and revert with a clear, specific error (e.g. Pot__NeverFunded()) if not rather than relying on the underlying ERC20 transfer to fail with a generic error.

**Finding 1 — Add an explicit allowance check before `transferFrom`:**
```diff
+ if (token.allowance(msg.sender, address(this)) < totalRewards) {
+ revert ContestManager__InsufficientAllowance();
+ }
token.transferFrom(msg.sender, address(pot), totalRewards);
```
**Finding 2 — Add a check confirming the Pot was actually funded before allowing `closePot()` to proceed, or provide a way to cancel an unfunded Pot:**
```diff
+ if (i_token.balanceOf(address(this)) == 0) {
+ revert Pot__NeverFunded();
+ }
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
...
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 4 hours 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!