Gauge Rewards Not Claimable By LP Token Stakers

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L472

Vulnerability details

Background

Based on the code of Gauge contract, there are two types of rewards that can be claimed from the Gauge contract by its users.

1) Gauge Rewards - For users (Liquidity providers) who deposit their LP tokens (a.k.a LP Token Staker) into the liquidity gauge. New VELO tokens will be minted weekly and portion of the newly minted VELO tokens will be allocated to the gauges. Coin rates which the gauge will be getting depends on the gauge weight. Each LP Token staker receives a share of newly minted VELO proportional to the amount of LP tokens locked. 2) Bribe Rewards - For veVELO token holders who voted for a given gauge with bribe rewards attached. They will be entitled a portion of the bribe rewards.

It appears that Velodrome's Gauge contract attempted to implement a single unified reward system for both LP Token Staker and veVELO holders for their Gauge Rewards and Bribe Rewards respectively.

Vulnerability Details

It was observed that a LP Token Staker will not be able to get their gauge rewards unless they hold veVELO token and has voted in a liquidity gauge.

This is an issue because a LP Token Staker should be able to get their gauge rewards regardless of whether they have voted or not. As far as I'm aware, all the major protocols do not require their LP Token Staker to vote in order to receive their gauge rewards. A LP Token Staker is not always a Voter, and vice versa.

Proof-of-Concept

The following show the Gauge.earned function, which is used to calculated the gauge rewards earned by LP Token Staker. The if (cp.voted) { condition will cause LP Token Stakers who did not vote to be not able to receive any gauge rewards as the following piece of code will not be reached/executed:

reward += cp0.balanceOf * (_rewardPerTokenStored1 - _rewardPerTokenStored0) / PRECISION;

Thus, the reward variable will always be 0.

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L472

// earned is an estimation, it won't be exact till the supply > rewardPerToken calculations have run
function earned(address token, address account) public view returns (uint) {
    uint _startTimestamp = Math.max(lastEarn[token][account], rewardPerTokenCheckpoints[token][0].timestamp);
    if (numCheckpoints[account] == 0) {
        return 0;
    }

    uint _startIndex = getPriorBalanceIndex(account, _startTimestamp);
    uint _endIndex = numCheckpoints[account]-1;

    uint reward = 0;

    if (_endIndex > 0) {
        for (uint i = _startIndex; i < _endIndex; i++) {
            Checkpoint memory cp0 = checkpoints[account][i];
            Checkpoint memory cp1 = checkpoints[account][i+1];
            (uint _rewardPerTokenStored0,) = getPriorRewardPerToken(token, cp0.timestamp);
            (uint _rewardPerTokenStored1,) = getPriorRewardPerToken(token, cp1.timestamp);
            // @audit - User who did not vote will not receive any reward, including LP Token stakers.
            if (cp0.voted) {
                reward += cp0.balanceOf * (_rewardPerTokenStored1 - _rewardPerTokenStored0) / PRECISION;
            }
        }
    }

    Checkpoint memory cp = checkpoints[account][_endIndex];
    uint lastCpWeeksVoteEnd = cp.timestamp - (cp.timestamp % (7 days)) + BRIBE_LAG + DURATION;
    if (block.timestamp > lastCpWeeksVoteEnd) {
        (uint _rewardPerTokenStored,) = getPriorRewardPerToken(token, cp.timestamp);
        // @audit - User who did not vote will not receive any reward, including LP Token stakers.
        if (cp.voted) {
            reward += cp.balanceOf * (rewardPerToken(token) - Math.max(_rewardPerTokenStored, userRewardPerTokenStored[token][account])) / PRECISION;
        }
    }

    return reward;
}

Recommended Mitigation Steps

Update the reward system in Gauge so that LP Token Staker who did not vote are still able to get their gauge rewards. Instead of having a single contract that handles both gauge and bribe rewards logic, consider segregating gauge and bribe reward logic into two separate contracts, similar to Solidly design.

In Solidy, bribe rewards are added to BaseV1-bribes.sol while gauge rewards are added to BaseV1-gauges.sol. Bribers call BaseV1-bribes.getReward() to get their bribe rewards while LP Token stakers call BaseV1-gauges.getReward() to get their gauge rewards. This approach supports clear separation of duties.

[pooltypes]

Reverted to design that's more similar to Solidly for prod

[GalloDaSballo]

Dup of #59