Minato7namikazi - logic bug in `notifyRewardAmount` function

[sherlock-admin4]

Minato7namikazi

High

logic bug in notifyRewardAmount function

Vulnerability Detail

function notifyRewardAmount(address token, uint amount) external lock {
    // ... (previous code)

    if (block.timestamp >= periodFinish[token]) {
        uint256 balanceBefore = IERC20(token).balanceOf(address(this));
        _safeTransferFrom(token, msg.sender, address(this), amount);
        uint256 balanceAfter = IERC20(token).balanceOf(address(this));
        amount = balanceAfter - balanceBefore;
        rewardRate[token] = amount / DURATION;
    } else {
        uint _remaining = periodFinish[token] - block.timestamp;
        uint _left = _remaining * rewardRate[token];
        require(amount > _left); 
        uint256 balanceBefore = IERC20(token).balanceOf(address(this));
        _safeTransferFrom(token, msg.sender, address(this), amount);
        uint256 balanceAfter = IERC20(token).balanceOf(address(this));
        amount = balanceAfter - balanceBefore;
        rewardRate[token] = (amount + _left) / DURATION;
    }

    // ... (remaining code)
}

The bug is in the else block. There's a check require(amount > _left);, but this check is performed before the actual token transfer. This means that the check is using the amount parameter passed to the function, not the actual amount of tokens received.

This can lead to a situation where the function might proceed even if the actual transferred amount is less than _left, which could result in incorrect reward rate calculations or even underflow in extreme cases.

To fix this, the check should be moved after the token transfer and use the actual received amount:

} else {
    uint _remaining = periodFinish[token] - block.timestamp;
    uint _left = _remaining * rewardRate[token];
    uint256 balanceBefore = IERC20(token).balanceOf(address(this));
    _safeTransferFrom(token, msg.sender, address(this), amount);
    uint256 balanceAfter = IERC20(token).balanceOf(address(this));
    amount = balanceAfter - balanceBefore;
    require(amount > _left, "Insufficient reward amount"); 
    rewardRate[token] = (amount + _left) / DURATION;
}

This will ensure that the check is performed on the actual amount of tokens received, rather than the amount claimed to be sent, providing better security & accuracy in reward calculations.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/FvmGauge.sol#L539

Tool used

Manual Review

Duplicate of #259