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

no approve to managercut

chenxingyu

Summary

The ContestManager contract in its current form attempts to transfer ERC-20 tokens to Pot contracts without first ensuring that the necessary approval has been granted. This is a critical issue because the ERC-20 standard requires that any spender must be pre-approved by the token owner before any transfer can be made using transferFrom.

Vulnerability Details

The fundContest function in the ContestManager contract is designed to transfer tokens from the owner to a Pot contract. However, it lacks the necessary approval step, which is a prerequisite for the transferFrom method to succeed. The transferFrom function in the ERC-20 standard (as implemented in OpenZeppelin) relies on an existing allowance set by the owner.

Impact

If the fundContest function attempts to transfer tokens without prior approval, the transaction will fail, resulting in a loss of functionality. This could lead to the inability to fund contests, thereby preventing the contract from being used as intended. This flaw could also lead to a loss of user funds if they are unable to retrieve their tokens after an unsuccessful transfer attempt.

Proof

The proof of this vulnerability lies in the interaction between the ContestManager contract and the ERC-20 token contract. Here is the relevant portion of the OpenZeppelin ERC-20 contract that shows how the transferFrom function operates:

function transferFrom(address from, address to, uint256 value) public virtual returns (bool) {
address spender = _msgSender();
_spendAllowance(from, spender, value);
_transfer(from, to, value);
return true;
}

And the _spendAllowance function that checks the allowance:

function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
uint256 currentAllowance = allowance(owner, spender);
if (currentAllowance != type(uint256).max) {
if (currentAllowance < value) {
revert ERC20InsufficientAllowance(spender, currentAllowance, value);
}
unchecked {
_approve(owner, spender, currentAllowance - value, false);
}
}
}

As seen, currentAllowance < value will cause the transaction to revert if there is not enough allowance. Since the ContestManager does not have a mechanism to set this allowance, any call to token.transferFrom will fail unless the owner has manually approved the ContestManager or Pot contract to spend their tokens, which is outside the scope of the current contract logic.

The _approve Function

The _approve function is responsible for updating the allowance mapping:

function _approve(address owner, address spender, uint256 value, bool emitEvent) internal virtual {
if (owner == address(0)) {
revert ERC20InvalidApprover(address(0));
}
if (spender == address(0)) {
revert ERC20InvalidSpender(address(0));
}
_allowances[owner][spender] = value;
if (emitEvent) {
emit Approval(owner, spender, value);
}
}

This function is crucial for updating the _allowances mapping, which is used to keep track of how much a spender is allowed to transfer on behalf of the owner.

Recommendations

  1. Implement an Approval Step: Before attempting to fund a Pot, add a function to the ContestManager that calls approve on the token contract to set the necessary allowance.

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.approve(msg.sender, address(pot), totalRewards)
token.transferFrom(msg.sender, address(pot), totalRewards);
​
}
function getContests() public view returns (address[] memory) {
return contests;
}
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.