burnerelu - RewardsDistributorV2 uses unsafe formulas to calculate balance

[sherlock-admin3]

burnerelu

Medium

RewardsDistributorV2 uses unsafe formulas to calculate balance

Summary

An unsafe formula is used in various places in RewardsDistributorV2.sol (here, here, here and here). Casting a negative int256 to an uint would create an undeflow, causing the Math.max function to return a very large number

Vulnerability Detail

The following formula is used throughout the RewardsDistributorV2.sol contract: Math.max(uint(int256(x)), 0) where x is something like: pt.bias - pt.slope * dt where dt is a time interval

This formula is supposed to return 0 when the balance is a negative number. However, if x is a negative number, uint(int256(x)) will produce a very large positive number.

Slope is initially calculated as the amount locked over a fixed int, the number of seconds in 52 weeks (< 10^9). The bias is initially calculated as slope times the delta between the lock end timestamp and block timestamp. This delta shall be named dt_bias for readability.

Looking at the _claim function, the relevant line looks like this: uint balance_of = Math.max(uint(int256(old_user_point.bias - dt * old_user_point.slope)), 0);

Knowing that the bias is also a function of the slope, this boils down to dt_bias - dt being less or greater than 0. The dt_bias is computed as following: dt_bias = int128(int256(old_locked.end - block.timestamp)); dt is computed as following: int128 dt = int128(int256(week_cursor - old_user_point.ts));

Making these functions return a wrong result is a matter of tuning the parameters in such a way to produce a negative result.

Impact

If I could reproduce this in a test, I would have marked it as a High. I only managed to produce a point where the bias and slope would be equal:

│   ├─ [906] VotingEscrow::point_history(3) [staticcall]
│   │   └─ ← [Return] Point({ bias: 2, slope: 2, ts: 1209599 [1.209e6], blk: 3 })

Producing a point with a bias equal to the slope means that (bias - dt * slope) becomes negative for any interval greater than one.

Due to the time limit and the desire to explore other defects, I decided to mark this as a medium.

Code Snippet

The following snippet which shows the behavior of casting a negative int256 to an uint has been tested on Remix, using the same solidity version 0.8.13:

function abc() public pure returns (uint) {
    return uint(int256(-1));
}

Output: { "0": "uint256: 115792089237316195423570985008687907853269984665640564039457584007913129639935" }

Tool used

Manual Review Remix

Recommendation

Simply compute x and return 0 if it's a negative number:

x = int256(old_user_point.bias - dt * old_user_point.slope);
return (x < 0) ? 0 : uint(x);
}

[nevillehuang]

See comment here