`checkedSub()` might revert due to underflow

[code423n4]

Lines of code

https://github.com/generationsoftware/pt-v5-twab-controller/blob/0145eeac23301ee5338c659422dd6d69234f5d50/src/libraries/OverflowSafeComparatorLib.sol#L55

Vulnerability details

Impact

checkedSub() might revert due to underflow and several internal functions calling _calculateTemporaryObservation() wouldn't work.

Proof of Concept

checkedSub() calculates the difference between two timestamps.

  function checkedSub(uint32 _a, uint32 _b, uint32 _timestamp) internal pure returns (uint32) {
    // No need to adjust if there hasn't been an overflow

    if (_a <= _timestamp && _b <= _timestamp) return _a - _b;

    uint256 aAdjusted = _a > _timestamp ? _a : _a + 2 ** 32;
    uint256 bAdjusted = _b > _timestamp ? _b : _b + 2 ** 32;

    return uint32(aAdjusted - bAdjusted);
  }

During calculation, it's possible to meet the underflow but it doesn't care at all.

It's from the Uniswap V3 Oracle but there is no problem because it uses the lower version than 0.8.0.

As a result, _extrapolateFromBalance() and several internal functions might revert.

  function _extrapolateFromBalance(
    ObservationLib.Observation memory _observation,
    uint32 _timestamp
  ) private pure returns (uint128 cumulativeBalance) {
    // new cumulative balance = provided cumulative balance (or zero) + (current balance * elapsed seconds)
    return
      _observation.cumulativeBalance +
      uint128(_observation.balance) *
      (_timestamp.checkedSub(_observation.timestamp, _timestamp)); //@audit revert
  }

Tools Used

Manual Review

Recommended Mitigation Steps

All calculations in checkedSub should be in unchecked block to work the same as Uniswap.

Assessed type

Under/Overflow

[Picodes]

How can it lead to an underflow with a == timestamp

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid