kirk-baird
This is a reentrancy vulnerability that would allow the attacker to drain the entire SHER balance of the contract.
Note: this attack requires gaining control of execution sher.transfer() which will depend on the implementation of the SHER token. Control may be gained by the attacker if the contract implements ERC777 or otherwise makes external calls during transfer().
See _sendSherRewards
function _sendSherRewardsToOwner(uint256 _id, address _nftOwner) internal {
uint256 sherReward = sherRewards_[_id];
if (sherReward == 0) return;
// Transfers the SHER tokens associated with this NFT ID to the address of the NFT owner
sher.safeTransfer(_nftOwner, sherReward);
// Deletes the SHER reward mapping for this NFT ID
delete sherRewards_[_id];
}
Here sherRewards are deleted after the potential external call is made in sher.safeTransfer(). As a result if an attacker reenters this function sherRewards_ they will still maintain the original balance of rewards and again transfer the SHER tokens.
As _sendSherRewardsToOwner() is internal the attack can be initiated through the external function ownerRestake() see here.
Steps to produce the attack:
n/a
Reentrancy can be mitigated by one of two solutions.
The first option is to add a reentrancy guard like nonReentrant the is used in SherlockClaimManager.sol.
The second option is to use the checks-effects-interactions pattern. This would involve doing all validation checks and state changes before making any potential external calls. For example the above function could be modified as follows.
function _sendSherRewardsToOwner(uint256 _id, address _nftOwner) internal {
uint256 sherReward = sherRewards_[_id];
if (sherReward == 0) return;
// Deletes the SHER reward mapping for this NFT ID
delete sherRewards_[_id];
// Transfers the SHER tokens associated with this NFT ID to the address of the NFT owner
sher.safeTransfer(_nftOwner, sherReward);
}
Additionally the following functions are not exploitable however should be updated to use the check-effects-interations pattern.
The text was updated successfully, but these errors were encountered:
All reactions