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

Unchecked transferFrom Operation in `ContestManager::fundContest`

Dec3mber

Summary

The ContestManager::fundContest function uses an unchecked transferFrom operation, which may lead to situations where tokens are not properly transferred to the Pot contract without the contract detecting the failure.

Vulnerability Details

TheContestManager::fundContest attempts to transfer tokens from the caller to a specific Pot contract using the following code:

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

However, this operation does not check the return value of ContestManager::token.transferFrom. If the operation fails (e.g., due to insufficient balance or allowance), the contract will proceed as if the transfer was successful, potentially leading to a mismatch between the expected and actual funds available in the Pot.

Impact

Funds Mismatch: The Pot contract may not receive the intended amount of tokens, leading to insufficient rewards available for distribution. This could cause users to attempt to claim rewards that do not exist, resulting in failed transactions and a poor user experience.

Tools Used

Manual Review

Slither

Recommendations

Use OpenZeppelin’s SafeERC20 library to ensure the transferFrom operation is checked for success, and revert the transaction if it fails.

function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]); //@audit everytime declare pot contract, why use a storage variable to store?
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);
+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);
}

Or check the return value.

function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]); //@audit everytime declare pot contract, why use a storage variable to store?
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);
+ (bool success, ) = token.safeTransferFrom(msg.sender, address(pot), totalRewards);
+ if (!success) {
+ revert();
+ }
}
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.