The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

During `claimRewards()` the ERC20 token transfer can fail in some edge cases without reverting, resulting in a loss of funds for the user

greed

Impact

When a user tries to claim its rewards using the LiquidationPool::claimRewards() function, in case the token transfer fails without reverting, the user won't be able to claim it again as the storage variable responsible for keeping track of the rewards will be deleted.

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L175

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L170

This will result in a loss of funds for the user.

Proof of concept

The function responsible for claiming the accrued awards is the following

function claimRewards() external {
ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager).getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
​
}
}

It goes through all the _tokens allowed by the protocol and if the user accrued rewards for the current token, the entry containing these informations is first deleted then the token transfer occures.

As ERC20 implementations differ from one another, the transfer may not revert in case it fails (it could just return false, for example) but the function will still exit successfully.

Since the mapping has been deleted, the rewards will be stuck in the contract.

Tools used

Manual analysis

Recommended mitigation steps

Use the safeTransfer() function from SafeERC20.sol just like in https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L154

IERC20(_token.addr).safeTransfer(msg.sender, _rewardAmount);
Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

unchecked-transfer

informational/invalid

Support

FAQs

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

Give us feedback!