bughuntoor - Rewards supplied to a gauge, prior to its first depositor will be permanently lost.

[sherlock-admin4]

bughuntoor

Medium

Rewards supplied to a gauge, prior to its first depositor will be permanently lost.

Summary

Rewards supplied to a gauge, prior to its first depositor will be permanently lost.

Vulnerability Detail

Every week, gauges receive rewards based on their pool weight, within the Voter contract.

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

The problem is that any rewards sent to the gauge prior to its first depositor will remain permanently stuck. Given that rewards are sent automatically, the likelihood of such occurrence is significantly higher

Impact

Loss of funds

Code Snippet

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

Tool used

Manual Review

Recommendation

Revert in case current supply is 0.

[nevillehuang]

@dawiddrzala Could you assist in verifying if this issue is valid? I initially thought it was invalid because it is unrealistic to deposit rewards when there is no depositors. However, given distribute is permissionless, could this be an issue?

[sherlock-admin2]

The protocol team fixed this issue in the following PRs/commits: https://github.com/Velocimeter/v4-contracts/pull/23

[Audinarey]

@WangSecurity

This impact alone I believe is low severity, I don't see it as a "loss of funds" or a "loss of yield".

...As I've said it's not a loss of funds because no one should get those rewards, including the protocol.

you mentioned here about a week ago that this is a low. How come this is a medium in this case as the scenario is about the same?

cc: @nevillehuang @cvetanovv

[spacegliderrrr]

There is loss of funds - tokens are stuck and no once can retrieve them. The tokens hold monetary value, therefore this is loss of funds.

Given that distribution happens both automatically and in a permissionless way, the likelihood of the vulnerability scales exponentially. Issue should remain as is.

[WangSecurity]

I agree with @spacegliderrrr here. The reward distribution on Velocimeter is automatic, while on Kwenta it required an admin to send the rewards.

Additionally, on the issue you mentioned, the problem was that the owner would send rewards before anyone stakes, which is admin mistake and we should assume it wouldn't happen. I didn't mention it initially because I understood it a bit later when the discussion on Kwenta stopped. Also, the issue required for all the stakers to withdraw from the contract. Here, the distribution is automatic and doesn't require any mistakes.

Also, for a detailed answer on Kwenta, look at the discussion under issue 94 where Watsons explained why in the context of Kwenta it was even better to keep these funds in the contract.

Hence, I agree it should remain as it is in the context of Velocimeter.

[spacegliderrrr]

Fix looks good. notifyRewardAmount now checks that totalSupply > 0

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.