Incorrect reward distribution when t == roundedTimestamp in RewardsDistributor

Summary

Vulnerability Detail

The RewardsDistributorV2 contract has a vulnerability that leads to incorrect reward distribution when a transaction occurs at the beginning of an epoch. The issue is related to the _checkpoint_total_supply() function, where the reward calculation may be inaccurate when the timestamp exactly matches the rounded timestamp. This can result in incorrect reward amounts for users.

RewardsDistributorV2 distributes newly minted tokens to users who lock the tokens in VotingEscrow. The RewardsDistributorV2 stores the supply in the public veSupply mapping. The RewardsDistributorV2 _checkpoint_total_supply() function iterates from the last updated time until the latest epoch time, fetches totalSupply from VotingEscrow, and saves it in veSupply.

Impact

The impact of this vulnerability is that users may receive incorrect rewards when interacting with the RewardsDistributorV2 contract. Specifically, when a transaction occurs at the beginning of an epoch and the timestamp matches the rounded timestamp, the reward calculation for subsequent transactions will be inaccurate.

Proof of Concept Assume the following scenario when a transaction is executed at the beginning of an epoch:

Alice locks 100 tokens, and the supply of flow/weth LPtoken increases to 100.
Bob calls the _checkpoint_total_supply(). The RewardsDistributor saves the veSupply as 100.
Bob locks 300 tokens, and the supply of lp token increases to 400.
After some time, Bob claims the reward. The reward is calculated by totalReward * balance / supply. However, due to the vulnerability, Bob gets reward = 3 instead of the expected reward = 3/4.
The invariance of the week-bound total supply is broken.

NOTE: This kind of vulnerability is common in similar type of codebases.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/RewardsDistributorV2.sol#L149

Tool used

Reference issue from velodrome

Recommendation

It is recommended to update the _checkpoint_total_supply() function as follows:


    function _checkpoint_total_supply() internal {
        address ve = voting_escrow;
        uint t = time_cursor;
        uint rounded_timestamp = block.timestamp / WEEK * WEEK;
        IVotingEscrow(ve).checkpoint();

        for (uint i = 0; i < 20; i++) {
-           if (t > rounded_timestamp) 
+           if (t >= rounded_timestamp) {
                break;
            } else {
                uint epoch = _find_timestamp_epoch(ve, t);
                IVotingEscrow.Point memory pt = IVotingEscrow(ve).point_history(epoch);
                int128 dt = 0;
                if (t > pt.ts) {
                    dt = int128(int256(t - pt.ts));
                }
                ve_supply[t] = Math.max(uint(int256(pt.bias - pt.slope * dt)), 0);
            }
            t += WEEK;
        }
        time_cursor = t;
    }

Duplicate of #495

sherlock-audit / 2024-06-velocimeter-judging

Chinmay - Incorrect reward distribution when t == roundedTimestamp in RewardsDistributor #595