sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

sonny2k - Unsafe casting in RewardsDistributorV2 leads to underflow of ve_for_at #578

Closed sherlock-admin3 closed 3 months ago

sherlock-admin3 commented 3 months ago

sonny2k

Medium

Unsafe casting in RewardsDistributorV2 leads to underflow of ve_for_at

Vulnerability Detail

Solidity does not revert when casting a negative number to uint. Instead, it underflows to a large number. In the RewardDistributor contract, the balance of a token at a specific time is calculated as follows:

     IVotingEscrow.Point memory pt = IVotingEscrow(ve).user_point_history(_tokenId, epoch);
     return Math.max(uint(int256(pt.bias - pt.slope * (int128(int256(_timestamp - pt.ts))))), 0);

This is supposed to return zero when the calculated balance is a negative number. However, it underflows to a large number.

Impact

This would lead to incorrect reward distribution if third-party protocols depend on this function, or when further updates make use of this codebase.

Code Snippet

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

Tool used

Manual Review

Recommendation

Recommend following other parts of the codebase and returning zero for a negative number.

     int256 result = int256(pt.bias - pt.slope * int128(int256(_timestamp - pt.ts)));
     if (result < 0) return 0;
     return uint256(result);

Also, recommend applying the fix to other parts of RewardsDistributorV2 as well

  1. https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/RewardsDistributorV2.sol#L208
  2. https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/RewardsDistributorV2.sol#L265
  3. https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/RewardsDistributorV2.sol#L158
nevillehuang commented 2 months ago

See comment here