Under current price implementation, the last day for a range will always use the interest from next range instead of its own interest.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/rwaOracles/RWADynamicOracle.sol#L151-L171
https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/rwaOracles/RWADynamicOracle.sol#L262-L274

Vulnerability details

Impact

Under current price implementation, the last day for a range will always use the interest from next range instead of its own interest.

Proof of Concept

In setRange() function, it will set prevClosePrice as below.

uint256 prevClosePrice = derivePrice(lastRange, lastRange.end - 1);

This will always set the prevClosePrice to the price of one day before range.end. For example, if the periodStart is July 31, 8 PM, and periodEnd is August 31, 8 PM, when we set next range as August 31, 8 PM to September 30, 8 PM, the prevClosePrice will be the price at August 30, 8 PM. Now if we set the timestamp to August 31, 8 PM, the daily IR for August 30, 8 PM to August 31, 8 PM will be the IR from August 31, 8 PM to September 30, 8 PM, this is incorrect since August 30 to August 31 is still within the first range.

A simple POC can explain, add it to RWADynamicOracle.t.sol

function testLastDayInRangeUsingWrongIR() public {
  // IR between August 31 8pm UTC to September 30 8pm UTC is dailyIR_oct
  oracleUSDY.setRange(periodEnd + 30 days, dailyIR_oct);

  // Set the timestamp to August 31 8pm UTC
  vm.warp(periodEnd);

  uint256 currentPrice = oracleUSDY.getPrice();

  // In theory, the correct price should be 1 * 1.00013368 ^ 31 = 1.0041524
  // However, the value we get is 1 * 1.00013368 ^ 30 * 1.00018538 = 1.004204308
  assertEq(currentPrice, 1004204300000000000);
} 

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

The issue here is due to using a prevClosePrice which is calculated as range initial price / dailyIR, I am not sure the reason about this since I think we can directly use that initial price as a start price.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as duplicate of #492

[c4-pre-sort]

raymondfam marked the issue as low quality report

[raymondfam]

Insufficient elaboration.

[c4-judge]

kirk-baird marked the issue as unsatisfactory:
Invalid