MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

`ContestManager::fundContest` can be called more than once leading to bigger distribution of rewards than intended

NHristov

Summary

The ContestManager contract has a vulnerability in the ContestManager::fundContest function. This function can be called multiple times, leading to a bigger distribution of rewards than intended after the reward distribution system has been closed.

Vulnerability Details

The fundContest function in the ContestManager contract allows the owner to fund a contest with the specified total rewards. However, there is no mechanism in place to prevent this function from being called multiple times for the same contest. As a result, each call to fundContest will transfer the total rewards amount to the contest's Pot contract, potentially leading to overfunding.

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

Proof Of Concept

After we create a pot in the contest manager and we call ContestManager::fundContest we will be able to call it again.

Place the following test into TestMyCut.t.sol

function testCanFundPotMultipleTimes() public mintAndApproveTokens {
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 4);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
assertEq(ERC20Mock(weth).balanceOf(contest), 4);
​
vm.startPrank(user);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
assertEq(ERC20Mock(weth).balanceOf(contest), 8);
}

Impact

If the fundContest function is called more than once for the same contest, it will result in the Pot contract receiving more tokens than initially intended. This can lead to:

  • Overfunding of the contest, locking excess funds in the Pot contract.

  • Discrepancies in reward distribution, as the additional funds may not be allocated as intended.

  • Inefficient use of resources, as excess funds could have been utilized elsewhere.

Tools Used

  • Solidity compiler

  • Manual code review

  • Foundry

Recommendations

To mitigate this vulnerability, implement a mechanism to ensure that the funding of the contest has happened only once. Can be achieve by:

  1. Recommended
    - Fund the pot contract after creating it in ContestManager::createContest function and remove ContestManager::fundConest function

function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
// the owner should allow the contest manager contract to manage token assets before creating the contest
token.transferFrom(msg.sender, address(pot), totalRewards);
return address(pot);
}

2 - Adding a funding status check within the function and a storage variable to track the funding status for the pot. Here is an updated version of the ContestManager contract with the recommended changes:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Pot} from "./Pot.sol";
import {Ownable} from "lib/openzeppelin-contracts/contracts/access/Ownable.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
contract ContestManager is Ownable {
address[] public contests;
mapping(address => uint256) public contestToTotalRewards;
mapping(address => bool) public contestFunded;
​
error ContestManager__InsufficientFunds();
error ContestManager__ContestAlreadyFunded();
​
constructor() Ownable(msg.sender) {}
​
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
contestFunded[address(pot)] = false; // Initialize funding status
return address(pot);
}
​
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
​
if (contestFunded[address(pot)]) {
revert ContestManager__ContestAlreadyFunded();
}
​
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
​
token.transferFrom(msg.sender, address(pot), totalRewards);
contestFunded[address(pot)] = true; // Mark the contest as funded
}
}
Updates

Lead Judging Commences

equious Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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