cryptic - Claiming rewards may DoS due to underflow from unsafe casting in `RewardsDistributorV2`

[sherlock-admin3]

cryptic

Medium

Claiming rewards may DoS due to underflow from unsafe casting in RewardsDistributorV2

Summary

Claiming rewards may DoS due to unsafe casting from int256 to uint256, which will underflow to uint max if int256 is negative. This will DoS claiming rewards for the respective token id.

Vulnerability Detail

The following function updates the total supply checkpoint, which will be used for distributing rewards:

RewardsDistributorV2.sol#L195-L215

        for (uint i = 0; i < 50; i++) {
            if (week_cursor >= _last_token_time) break;

            if (week_cursor >= user_point.ts && user_epoch <= max_user_epoch) {
                user_epoch += 1;
                old_user_point = user_point;
                if (user_epoch > max_user_epoch) {
                    user_point = IVotingEscrow.Point(0,0,0,0);
                } else {
                    user_point = IVotingEscrow(ve).user_point_history(_tokenId, user_epoch);
                }
            } else {
                int128 dt = int128(int256(week_cursor - old_user_point.ts));
@>              uint balance_of = Math.max(uint(int256(old_user_point.bias - dt * old_user_point.slope)), 0); //@audit can underflow to uint max
                if (balance_of == 0 && user_epoch > max_user_epoch) break;
                if (balance_of != 0) {
@>                  to_distribute += balance_of * tokens_per_week[week_cursor] / ve_supply[week_cursor]; //@audit will DoS
                }
                week_cursor += WEEK;
            }
        }

        user_epoch = Math.min(max_user_epoch, user_epoch - 1);
        user_epoch_of[_tokenId] = user_epoch;
        time_cursor_of[_tokenId] = week_cursor;

Looking at the following line:

uint balance_of = Math.max(uint(int256(old_user_point.bias - dt * old_user_point.slope)), 0);

If int256(old_user_point.bias - dt * old_user_point.slope) < 0, then casting to a uint will underflow to type(uint).max, causing DoS either due to to_distribute calculation reverting from overflow or insufficient funds when transferring the rewards to the user.

In this case, the user_epoch_of and time_cursor_of mappings will not be updated, so it will continue to use the old_user_point values and consistently DoS due to overflow, making rewards unclaimable for the token id.

Impact

Unclaimable rewards, denial of service, loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/RewardsDistributorV2.sol#L142-L163

Tool used

Manual Review

Recommendation

Consider making the following changes in RewardsDistributorV2::_claim and all other instances where Math.max(uint(int256...), 0) is used:

        for (uint i = 0; i < 50; i++) {
            if (week_cursor >= _last_token_time) break;

            if (week_cursor >= user_point.ts && user_epoch <= max_user_epoch) {
                user_epoch += 1;
                old_user_point = user_point;
                if (user_epoch > max_user_epoch) {
                    user_point = IVotingEscrow.Point(0,0,0,0);
                } else {
                    user_point = IVotingEscrow(ve).user_point_history(_tokenId, user_epoch);
                }
            } else {
                int128 dt = int128(int256(week_cursor - old_user_point.ts));
+               uint balance_of;
+               int256 result = int256(old_user_point.bias - dt * old_user_point.slope);
+               if(result < 0) balance_of = 0;
+               else balance_of = uint(result);
-               uint balance_of = Math.max(uint(int256(old_user_point.bias - dt * old_user_point.slope)), 0);
                if (balance_of == 0 && user_epoch > max_user_epoch) break;
                if (balance_of != 0) {
                    to_distribute += balance_of * tokens_per_week[week_cursor] / ve_supply[week_cursor];
                }
                week_cursor += WEEK;
            }
        }

        user_epoch = Math.min(max_user_epoch, user_epoch - 1);
        user_epoch_of[_tokenId] = user_epoch;
        time_cursor_of[_tokenId] = week_cursor;

[nevillehuang]

See comment here