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

Use `safeTransfer` instead of `transfer` when interacting with arbitrary ERC-20 tokens

cosine

Summary

The claimRewards function inside the LiquidationPool contract can be used by stakers to claim their rewards. These rewards can be arbitrary ERC-20 tokens that were used as collateral. It uses the transfer function to send the tokens to the user. This function does not check the return value of the transfer function. This could lead to loss of funds for the user if the token does not implement the ERC-20 specification correctly.

Vulnerability Details

Here we can see that the claimRewards function uses transfer instead of safeTransfer:

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

Tokens not compliant with the ERC20 specification could return false from the transfer function call to indicate the transfer fails, while the calling contract would not notice the failure if the return value is not checked. Therefore, not using safeTransfer here could lead to loss of funds for the user. As the rewards are deleted right before the transfer and therefore the user is not able to claim them again in case of a failure.

Impact

Loss of user funds.

Recommendations

Use safeTransfer instead of transfer and allow users to claim specific rewards in case one is failing constantly.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year 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.