sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

feelereth - integer underflow vulnerability due to the use of uint96 for storing timestamps. #103

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

feelereth

high

integer underflow vulnerability due to the use of uint96 for storing timestamps.

Summary

There is a potential integer underflow vulnerability due to the use of uint96 for storing timestamps.

Vulnerability Detail

This Link 1 casts the uint256 timestamp returned by newProvider.current() into a uint96, which can cause underflow if the timestamp is greater than 2^96 - 1. Here Link 2 a uint256 timestamp is compared against a uint96 stored timestamp. If underflow occurred, this comparison may not work as expected.

A vulnerable exploit would be:

  1. Deploy a malicious oracle that returns a timestamp of 2^256 - 1
  2. Call update() to set this oracle
  3. The uint96 timestamp will underflow to 0
  4. Call at() with a timestamp between 0 and 2^96 - 1. It will incorrectly return the latest value instead of reverting. This allows an attacker to manipulate the return values of at().

Impact

An attacker could trick the contract into using incorrect historical values for critical operations like pricing.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L78 https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L68

Tool used

Manual Review

Recommendation

  1. Use uint256 instead of uint96 for storing timestamps
  2. Explicitly cast timestamps to uint256 before comparisons:
sherlock-admin commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

141345 commented:

x

darkart commented:

What Watson is saying makes no sense

panprog commented:

invalid because oracle is set by owner who is trusted