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

ContestManager::fundContest has no already-funded guard, so a repeated call pulls another full totalRewards into the Pot, where Pot's accounting ignores it and no function can ever return it to the owner

friedsocks233

No "already funded" guard in fundContest + duplicate funding permanently stranded

Description

  • The owner creates a contest and calls fundContest(index) once, which moves exactly totalRewards into the Pot. The Pot then pays out totalRewards through claims and the close.

  • fundContest writes no state and does not compare the Pot's balance with totalRewards, so every call transfers another full totalRewards.

  • Pot accounts only through remainingRewards. That value is set once from totalRewards in the constructor, and the Pot never reads its own token balance. Neither contract has a sweep or refund.

  • So the surplus sits outside every payout formula and can never go back to the owner.

// src/ContestManager.sol
@> 28: function fundContest(uint256 index) public onlyOwner {
29: Pot pot = Pot(contests[index]);
30: IERC20 token = pot.getToken();
31: uint256 totalRewards = contestToTotalRewards[address(pot)];
32:
33: if (token.balanceOf(msg.sender) < totalRewards) {
34: revert ContestManager__InsufficientFunds();
35: }
36:
@> 37: token.transferFrom(msg.sender, address(pot), totalRewards);
38: }
// src/Pot.sol
@> 27: remainingRewards = totalRewards;

Risk

Likelihood:

  • The owner repeats fundContest for an index while holding a standing allowance of at least twice totalRewards, for example a max approval. Typical causes are a retried transaction sent with a new nonce, a funding script that re-runs, or an index mix-up across several pots. With an exact approval, the second call reverts inside the token (ERC20InsufficientAllowance), not because of any guard in the protocol.

  • Only the trusted owner can trigger it, so it takes an operational mistake by a trusted role.

Impact:

  • A full duplicate of totalRewards is lost to the owner. In the PoC the owner is down 2000 for a 1000 contest. After every player claims and the pot is closed twice, the Pot still holds 1000 while getContestRemainingRewards reports 0.

  • When some rewards are unclaimed, repeated closes (the separate non-terminal-close issue) can push the surplus out, but only to claimants and to ContestManager, where it is locked. In the PoC five closes paid p1 500 + 5 x 225 = 1625 and locked 250 in ContestManager. None of the surplus ever returns to the owner.

Proof of Concept

The test deploys ContestManager and the repo's test/ERC20Mock.sol. All three tests pass:

  • test_baseline_exactApproval_secondFundRevertsOnAllowanceOnly: with an exact approval, the second fundContest(0) reverts only with ERC20InsufficientAllowance(conMan, 0, 1000).

  • test_doubleFund_surplusPermanentlyStuck:

    • With a max approval, two fundContest(0) calls leave 2000 in the Pot, while both views still say 1000.

    • Both players claim 500. Two closes move nothing.

    • The Pot keeps 1000, getContestRemainingRewards returns 0, and the owner's balance is START - 2000. The admin cannot call pot.closePot() directly.

  • test_doubleFund_surplusLeaksOnlyToClaimantsAndManager: p2 never claims, and five closes pay p1 1625 in total and lock 250 in ContestManager. The owner recovers nothing.

Save the file as test/PoC_MC-05.t.sol and run from the repository root:

FOUNDRY_FFI=false forge test --match-path test/PoC_MC-05.t.sol -vv
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test, console} from "lib/forge-std/src/Test.sol";
import {IERC20Errors} from "lib/openzeppelin-contracts/contracts/interfaces/draft-IERC6093.sol";
import {ContestManager} from "../src/ContestManager.sol";
import {Pot} from "../src/Pot.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
​
/// MC-05: fundContest has no "already funded" guard. A second call on the same index pulls another
/// full totalRewards into the Pot. Pot accounts only through remainingRewards (fixed at totalRewards),
/// so the surplus is outside every payout formula and no function ever returns it to the owner.
contract PoC_MC05 is Test {
address admin = makeAddr("admin");
address p1 = makeAddr("p1");
address p2 = makeAddr("p2");
​
uint256 constant TOTAL = 1000;
uint256 constant START = 1_000_000;
​
ContestManager conMan;
ERC20Mock token;
​
function setUp() public {
vm.startPrank(admin);
conMan = new ContestManager();
token = new ERC20Mock("TKN", "TKN", admin, 0);
token.mint(admin, START);
vm.stopPrank();
}
​
function _twoPlayerPot() internal returns (Pot pot) {
address[] memory players = new address[](2);
players[0] = p1;
players[1] = p2;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 500;
rewards[1] = 500;
vm.prank(admin);
pot = Pot(conMan.createContest(players, rewards, token, TOTAL));
}
​
/// Baseline / precondition: with an exact approval the duplicate call is stopped only by the
/// ERC20 allowance, not by any guard in ContestManager.
function test_baseline_exactApproval_secondFundRevertsOnAllowanceOnly() public {
Pot pot = _twoPlayerPot();
vm.startPrank(admin);
token.approve(address(conMan), TOTAL);
conMan.fundContest(0);
assertEq(token.balanceOf(address(pot)), TOTAL, "pot funded once");
​
vm.expectRevert(
abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(conMan), 0, TOTAL)
);
conMan.fundContest(0);
vm.stopPrank();
}
​
/// Main: with a standing (max) approval the second call succeeds, the pot holds 2x totalRewards,
/// every intended payout completes, and a full totalRewards is left stranded forever.
function test_doubleFund_surplusPermanentlyStuck() public {
Pot pot = _twoPlayerPot();
vm.startPrank(admin);
token.approve(address(conMan), type(uint256).max);
conMan.fundContest(0);
conMan.fundContest(0); // retry / script bug: no "already funded" guard
vm.stopPrank();
​
assertEq(token.balanceOf(address(pot)), 2 * TOTAL, "pot holds 2x totalRewards");
assertEq(token.balanceOf(admin), START - 2 * TOTAL, "owner paid twice");
assertEq(conMan.getContestTotalRewards(address(pot)), TOTAL, "bookkeeping still says 1000");
assertEq(pot.getRemainingRewards(), TOTAL, "Pot accounting ignores the surplus");
​
// Every player claims their full reward.
vm.prank(p1);
pot.claimCut();
vm.prank(p2);
pot.claimCut();
assertEq(token.balanceOf(p1), 500);
assertEq(token.balanceOf(p2), 500);
assertEq(pot.getRemainingRewards(), 0, "accounting says pot is empty");
​
// Close after the claim window (and again) -- remainingRewards == 0, so nothing moves.
vm.warp(block.timestamp + 91 days);
vm.startPrank(admin);
conMan.closeContest(address(pot));
conMan.closeContest(address(pot));
vm.stopPrank();
​
console.log("pot token balance after full payout + close:", token.balanceOf(address(pot)));
console.log("pot remainingRewards:", pot.getRemainingRewards());
console.log("owner balance (started 1,000,000):", token.balanceOf(admin));
​
// The duplicate totalRewards is stranded: accounting reports 0 while the pot holds 1000.
assertEq(token.balanceOf(address(pot)), TOTAL, "duplicate funding stranded in Pot");
assertEq(conMan.getContestRemainingRewards(address(pot)), 0, "views report nothing left");
assertEq(token.balanceOf(address(conMan)), 0);
assertEq(token.balanceOf(admin), START - 2 * TOTAL, "owner never recovers the surplus");
​
// The owner has no direct path: Pot's only owner is ContestManager, and ContestManager
// exposes no sweep/withdraw (closeContest is the only Pot-facing write).
assertEq(pot.owner(), address(conMan));
vm.prank(admin);
vm.expectRevert();
pot.closePot();
}
​
/// Limb 2: when some rewards are unclaimed, repeated closes (no closed flag, remainingRewards
/// never decremented in closePot) can push part of the surplus out -- but only to claimants and
/// to ContestManager (which has no withdraw). None of it returns to the owner.
function test_doubleFund_surplusLeaksOnlyToClaimantsAndManager() public {
Pot pot = _twoPlayerPot();
vm.startPrank(admin);
token.approve(address(conMan), type(uint256).max);
conMan.fundContest(0);
conMan.fundContest(0);
vm.stopPrank();
​
vm.prank(p1);
pot.claimCut(); // p2 never claims; remainingRewards = 500
​
vm.warp(block.timestamp + 91 days);
uint256 closes;
vm.startPrank(admin);
// each close pays managerCut 50 + claimantCut (450/2 = 225) = 275 while the pot can cover it
while (token.balanceOf(address(pot)) >= 275) {
conMan.closeContest(address(pot));
closes++;
}
vm.stopPrank();
​
console.log("closes executed:", closes);
console.log("p1 total received:", token.balanceOf(p1));
console.log("ContestManager locked balance:", token.balanceOf(address(conMan)));
console.log("pot residue:", token.balanceOf(address(pot)));
console.log("owner balance (started 1,000,000):", token.balanceOf(admin));
​
// Single-funding intent: p1 gets 500 + 225, manager gets 50, 225 residue.
// Double funding lets 5 closes run: p1 gets 500 + 5*225, manager gets 5*50.
assertEq(closes, 5);
assertEq(token.balanceOf(p1), 500 + 5 * 225);
assertEq(token.balanceOf(address(conMan)), 5 * 50);
assertEq(token.balanceOf(address(pot)), 2 * TOTAL - 500 - 5 * 275);
assertEq(token.balanceOf(admin), START - 2 * TOTAL, "owner recovers nothing");
}
}

Recommended Mitigation

Record that a pot has been funded and reject a second funding.

--- a/src/ContestManager.sol
+++ b/src/ContestManager.sol
@@ -8,8 +8,10 @@
contract ContestManager is Ownable {
address[] public contests;
mapping(address => uint256) public contestToTotalRewards;
+ mapping(address => bool) public contestFunded;
error ContestManager__InsufficientFunds();
+ error ContestManager__AlreadyFunded();
constructor() Ownable(msg.sender) {}
@@ -27,6 +29,10 @@
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
+ if (contestFunded[address(pot)]) {
+ revert ContestManager__AlreadyFunded();
+ }
+ contestFunded[address(pot)] = true;
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
Updates

Lead Judging Commences

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