MyCut

Summary

The fundContest() function in the ContestManager contract can be called multiple times for the same contest, which can result in overfunding. There is no mechanism to check whether the contest has already been funded, leading to repeated transfers of the totalRewards amount from the owner to the Pot contract.

Vulnerability Details

In the fundContest() function, the owner is responsible for transferring the total rewards for a contest to the associated Pot contract:

However, this function can be called multiple times for the same contest. Each time the function is called, the owner will send the full totalRewards amount again, regardless of whether the contest has already been fully funded. This can lead to the following issues:

1. Overfunding: Multiple calls to fundContest() result in the Pot receiving more tokens than intended.

2. Locked Excess Funds: Since there is no way to withdraw funds from the Pot contract, the excess tokens transferred due to overfunding become locked, potentially leading to a permanent loss of tokens.

Example Scenario:

1. Initial Setup:

   • A contest is created with 1000 tokens in total rewards.

2. Scenario:

   • The owner calls fundContest() to fund the contest with 1000 tokens.

   • The owner mistakenly calls fundContest() again for the same contest, transferring an additional 1000 tokens.

3. Impact:

   • The Pot contract now holds 2000 tokens, even though only 1000 tokens are required for the rewards.

   • The excess 1000 tokens are locked in the Pot contract with no mechanism to retrieve them.

Impact

1. Overfunding: Multiple calls result in the Pot receiving more tokens than needed, which wastes resources and leads to unintended transfers.

2. Locked Funds: Since the excess tokens cannot be withdrawn from the Pot contract, they become permanently locked and unusable.

Steps to Reproduce the Issue:

1. Step 1: Deploy the ContestManager contract and create a contest with a reward pool.

2. Step 2: Call fundContest() to fund the contest.

3. Step 3: Call fundContest() again for the same contest.

4. Expected Outcome: The contract should prevent multiple calls or at least check if the contest is already funded.

5. Actual Outcome: The contract allows multiple transfers, leading to overfunding and locked excess tokens.

Tools Used

Manual Review

Recommendations

To prevent multiple calls from leading to overfunding, add a check to ensure that the contest is not funded more than once. This can be achieved by introducing a flag that tracks whether a contest has already been funded or by checking the balance of the Pot contract before allowing additional transfers.

Steps to Fix:

1. Introduce a Flag to Track Funding: Add a boolean flag isFunded in the contestToTotalRewards mapping to track whether a contest has already been funded:

   mapping(address => bool) public isContestFunded;

2. Modify the fundContest() Function: Check if the contest has already been funded before allowing the transfer:

   function fundContest(uint256 index) public onlyOwner {

   Pot pot = Pot(contests[index]);

   + require(!isContestFunded[address(pot)], "Contest already funded");

   ​

   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);

   ​

   // Mark the contest as funded

   + isContestFunded[address(pot)] = true;

   }