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

M-01: Silent Fund Transfer Failure in `ContestManager::fundContest`

syedrabeet2002

Summary

This vulnerability arises when the ContestManager::fundContest function fails to verify the success of token transfers using transferFrom. If the transfer fails silently, the contract mistakenly considers the contest pot as funded, leading to potential underfunding and state inconsistencies.

Vulnerability Details

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

The fundContest function calls the transferFrom function to move tokens from the contract owner to the contest's Pot. However, it does not check the return value of transferFrom, which indicates whether the transfer was successful (true) or failed (false).

Add this function to your ERC20Mock contract before running the test case.

function transferFrom(address sender, address recipient, uint256 amount) public override returns (bool) {
// Simulate failure
return false;
}
​

Add this test case to you TestMyCut file.

function testSilentlyFailing() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 100;
contest = ContestManager(conMan).createContest(
players,
rewards,
IERC20(ERC20Mock(weth)),
totalRewards
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
}

The failure to check the return value of transferFrom in the fundContest function creates a vulnerability where token transfers can fail silently. This leads to a mismatch between the contract's internal state and actual token balances, causing underfunded contests and potentially significant financial discrepancies. Properly handling the return value would prevent this issue, ensuring that contests are only marked as funded when the transfer has been successfully completed.

Impact

  • Participants may attempt to claim rewards from contests that are incorrectly marked as funded, leading to disputes and dissatisfaction when they discover that no tokens are available.

  • Repeated occurrences of such issues can undermine trust in the platform or smart contract, causing users to lose confidence in its reliability and fairness.

  • The contract's internal records may show contests as fully funded, while the actual token balances are insufficient, leading to significant financial mismatches that could affect the project's overall integrity.

Tools Used

Manual Review

Recommendations

Do this modification in your ContestManager contract

+++ error ContestManager__TransferFailed();
​
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();
}
​
+++ bool success = token.transferFrom(msg.sender, address(pot), totalRewards);
+++ if (!success) {
+++ revert ContestManager__TransferFailed();
+++ }
}
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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