Wrong calculation of elapsed days

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L266

Vulnerability details

Impact

For a certain period of time, the dailyIr is compounded every day. However, when calculating prevClosePrice, the last day's addition is missed.

The formula for calculating the current price is as follows: (Range.dailyInterestRate ** (Days Elapsed + 1)) * Range.lastSetPrice. Here, "Days Elapsed" refers to all the days that have passed during this time period.

For example, if the period starts on July 31 at 8:00:00 PM UTC (timestamp = 1690833600) and ends on August 31 at 8:00:00 PM UTC (timestamp = 1693512000), a total of 31 days have passed.

In the getPrice function, when range.end <= block.timestamp is true, the calculation of prevClosePrice calls derivePrice(range, range.end - 1);. Subtracting 1 from range.end (for your example periodEnd = 1693512000), the elapsed days will be 30 due to precision errors in:

uint256 elapsedDays = (currentTime - currentRange.start) / DAY;

A precision error causes the elapsed days to be 30 and the last dailyIr will not be compounded because the elapsed days will be one day less.

PoC

  function test_diffPrices() public {
    /*
      uint256 periodStart = 1690833600; // JUL 31 8:00:00PM UTC
      uint256 periodEnd = 1693512000; // AUG 31  8:00:00PM UTC
      uint256 dailyIR = 1.00013368 * 1e27; // (1 + Daily IR, scaled to 1e27),
      uint256 dailyIR_oct = 1.00018538 * 1e27;
      uint256 rangeStartPrice = 1e18; // Range Initial price
   */

    oracleUSDY = new RWADynamicOracle(
      address(this),
      address(this),
      address(this),
      periodStart,
      periodEnd, // range.end
      dailyIR,
      rangeStartPrice
    );

    uint256 price1 = oracleUSDY.simulateRange(
        periodStart,
        dailyIR,
        periodEnd - 1, // simulating block.timestamp
                       // if (range.end <= block.timestamp) -> false
                       // return derivePrice(range, block.timestamp);
        periodStart,
        rangeStartPrice
      );
      uint256 price2 = oracleUSDY.simulateRange(
        periodStart,
        dailyIR,
        periodEnd + 110, // simulating block.timestamp 
                         // if (range.end <= block.timestamp) -> true
                         // return derivePrice(range, range.end - 1); 
        periodStart,
        rangeStartPrice
      );

      console.log("The derived price is: ", price2);

      assertEq(price1, price2); // prices should not be equal, elapsedDays1 == elapsedDays2
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Do not substract 1 from range.end

Assessed type

Math

[raymondfam]

range.end - 1 is in terms of seconds whereas Days Elapsed + 1 is in terms of days.

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L266

    uint256 elapsedDays = (currentTime - currentRange.start) / DAY;

elapsedDays will have 1 day short but it's incremented here:

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L270

          _rpow(currentRange.dailyInterestRate, elapsedDays + 1, ONE),

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[kirk-baird]

Agreed with @raymondfam that 1 is added to elapsedDays to account for rounding down. The calculations are performed correctly and so I'm invalidating this issue.

[c4-judge]

kirk-baird marked the issue as unsatisfactory:
Invalid