Summary
Failed transfers of ERC-20 token's may result in loss of the admin or users funds.
Vulnerability Details
The Pot::_transferReward, Pot::closePot, and ContestManager::fundContest methods allow ERC-20 tokens to be transferred between accounts using the ERC-20 methods transfer and transferFrom. However, different ERC-20 tokens handle success or failure differently. Some revert the transaction on failure, while others simply return False. On success, some return True, while others don't return any value. The issue in the audited contracts is that there is no mechanism to check if these method calls succeeded or failed.
In Pot.sol file:
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
@> i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
function _transferReward(address player, uint256 reward) internal {
@> i_token.transfer(player, reward);
}
In ContestManager.sol file:
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);
}
Impact
If the transfer fails, the transaction will still be successful. Users won't claim their rewards or the admin won't fund the pool.
Tools Used
Recommended Mitigation
A mechanism should be implemented to check whether and what data is returned by the methods used to transfer tokens (transfer/transferFrom). It can be implemented by using the OpenZeppelin SafeERC20 library and using the safeTransfer and safeTransferFrom methods.
In Pot.sol file:
+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
+using SafeERC20 for IERC20;
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
- i_token.transfer(msg.sender, managerCut);
+ i_token.safeTransfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
function _transferReward(address player, uint256 reward) internal {
- i_token.transfer(player, reward);
+ i_token.safeTransfer(player, reward);
}
In ContestManager.sol file:
+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
+using SafeERC20 for IERC20;
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);
+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);
}
