sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

bareli - distribute will fail in "voter contract". #568

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

bareli

Medium

distribute will fail in "voter contract".

Summary

while calling the distribute function we are calling "IGauge(_gauge).notifyRewardAmount(base, _claimable)" which consist a lock modifier as distribute also consists of a lock modifier.This will cause notifyRewardAmount to fail.

Vulnerability Detail

function distribute(address _gauge) public lock { IMinter(minter).update_period(); _updateFor(_gauge); // should set claimable to 0 if killed uint _claimable = claimable[_gauge]; if (_claimable > IGauge(_gauge).left(base) && _claimable / DURATION > 0) { claimable[_gauge] = 0; if((_claimable * 1e18) / currentEpochRewardAmount > minShareForActiveGauge) { activeGaugeNumber += 1; }

@>> IGauge(_gauge).notifyRewardAmount(base, _claimable); emit DistributeReward(msg.sender, _gauge, _claimable); } }

As we can see notifyRewardAmount in the gauge contract.

@>> function notifyRewardAmount(address token, uint amount) external lock { require(token != stake); require(amount > 0); if (!isReward[token]) { require(IVoter(voter).isWhitelisted(token), "rewards tokens must be whitelisted"); require(rewards.length < MAX_REWARD_TOKENS, "too many rewards tokens"); } if (rewardRate[token] == 0) _writeRewardPerTokenCheckpoint(token, 0, block.timestamp); (rewardPerTokenStored[token], lastUpdateTime[token]) = _updateRewardPerToken(token, type(uint).max, true);

    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;
    }
    require(rewardRate[token] > 0);
    uint balance = IERC20(token).balanceOf(address(this));
    require(rewardRate[token] <= balance / DURATION, "Provided reward too high");
    periodFinish[token] = block.timestamp + DURATION;
    if (!isReward[token]) {
        isReward[token] = true;
        rewards.push(token);
    }

    emit NotifyReward(msg.sender, token, amount);
}

Impact

this will cause notifyRewardAmount to revert.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/Voter.sol#L559 https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/Gauge.sol#L516

Tool used

Manual Review

Recommendation

_unlocked = 1; IGauge(_gauge).notifyRewardAmount(base, _claimable); _unlocked = 2;

nevillehuang commented 3 months ago

Invalid, lock() is a reentrancy lock modifier, there will be no reverts