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

Missing ReEntrancy Guard in `claimRewards` function

KeyKiril

Summary

While true ERC-20 tokens that adhere to the specification and have undergone auditing are generally not susceptible to re-entry risks, caution should be exercised. Malicious actors can potentially create ERC-20 tokens with custom safeTransferFrom() or approve() functions containing re-entrancy hooks, posing a threat to the target.

Moreover, it's worth nothing that ERC-777, although offering improved usability, introduces transfer hooks that may introduce re-entrancy vulnerabilities. Therefore, careful consideration and appropriate security measures should be taken when dealing with ERC-777 tokens to mitigate the risk of re-entry attacks.

Vulnerability Details

While traditional ERC-20 tokens are designed to mitigate re-entrancy risks, ERC-777 tokens, due to their increased functionality and transfer hooks, can introduce potential re-entrancy vulnerabilities.

In scenarios where a contract interacts with unknown ERC-20 tokens, adopting a cautious approach and considering the possibility of re-entrancy problems is a prudent security measure. Vigilance and adherence to best practices are essential when dealing with diverse token standards to ensure the robustness of smart contract systems.

Add Re-Entrancy Guard in the following function:

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

Impact

Even though assets are not expected to re-enter and reward tokens re-entering on claimRewards might not be problematic.
Consider doing this change for security point of view.

Tools Used

Manual Review

Recommendations

Use Openzeppelin or Solmate Re-Entrancy pattern

Here is a example of a re-entracy guard:

pragma solidity 0.8.13;
​
contract ReEntrancyGuard {
bool internal locked;
​
modifier noReentrant() {
require(!locked, "No re-entrancy");
locked = true;
_;
locked = false;
}
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

claims-reentrancy

informational/invalid

Support

FAQs

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