apajaresaguilera - Prevent reentrancy

[sherlock-admin]

apajaresaguilera

low

Prevent reentrancy

Summary

A reentrancy attack can be performed if the reward token is updated to be an ERC777 token.

Impact

Low

Vulnerability Detail

A lack of reentrancy preventions allow users to reenter relevant protocol functions, such as claimRewards() in BasePool.sol, or withdraw() in TimeLockPool.sol .
If the reward token is updated to be an ERC777 token in the future (which has support for hooks), a malicious party could re-enter the function that performed the token transfer by calling the function transferring the tokens again in the ERC777 hook.
Especially in claimRewards(), where users obtain the rewards corresponding to their staking amount, the transfer could call a callback triggering an attacker's malicious contract function where claimRewards() could be called again.

Code Snippet

function claimRewards(address _receiver) external {
        uint256 rewardAmount = _prepareCollect(_msgSender());
        uint256 escrowedRewardAmount = rewardAmount * escrowPortion / 1e18;
        uint256 nonEscrowedRewardAmount = rewardAmount - escrowedRewardAmount;

        if(escrowedRewardAmount != 0 && address(escrowPool) != address(0)) {
            escrowPool.deposit(escrowedRewardAmount, escrowDuration, _receiver);
        }
        // ignore dust
        if(nonEscrowedRewardAmount > 1) {
            rewardToken.safeTransfer(_receiver, nonEscrowedRewardAmount);
        }
        emit RewardsClaimed(_msgSender(), _receiver, escrowedRewardAmount, nonEscrowedRewardAmount);
    }

Tool used

Manual Review

Recommendation

Add reentrancy guards to functions transferring reward tokens back from the protocol (claimRewards() in BasePool.sol, or withdraw() in TimeLockPool.sol) (see ReentrancyGuard.sol)