The Standard

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

LiquidationPool.claimRewards does not utilize safeTransfer

Summary

LiquidationPool.claimRewards() does not utilize safeTransfer() and instead utilizes transfer() when transferring funds to claimer.

Vulnerability Details

The LiquidationPool.claimRewards() function utilizes ERC20.transfer(), which is inappropriate to use. This can be seen in the claimRewards() function below:

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 {
// AUDIT: don't use transfer. Use safeTransfer.
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
}
}

Because there are no checks done on the response of the transfer() function, the protocol is mistakenly assuming that all tokens will revert on fail, which is not always the case.

Impact

If for whatever reason the ERC20.transfer() function returns a boolean value determining whether the transaction succeeded or not, the protocol will not handle an edge case where the transfer() function failed. This will lead to the protocol assuming that the reward was distributed when it really wasn't.

Tools Used

Manual review

Recommendations

Utilize safeTransfer() instead:

IERC20(_token.addr).safeTransfer(msg.sender, _rewardAmount);
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.