code-423n4 / 2023-10-opendollar-findings

10 stars 7 forks source link

TWAP can lead to loss of manipulation of price #431

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/oracles/UniV3Relayer.sol#L74 https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/oracles/CamelotRelayer.sol#L68

Vulnerability details

Impact

There are tradeoffs when choosing the length of the period of time to calculate a TWAP. Longer periods are better to protect against price manipulation, but come at the expense of a slower, and potentially less accurate, price.

Proof of Concept

Both the UniV3Relayer.sol and CamelotRelayer.sol contract have getResultWithValidity() function that uses TWAP Mechanism that can lead to loss of funds during unfavourable conditions.

  function getResultWithValidity() external view returns (uint256 _result, bool _validity) {
    // If the pool doesn't have enough history return false
    if (OracleLibrary.getOldestObservationSecondsAgo(camelotPair) < quotePeriod) {
      return (0, false);
    }
    // Consult the query with a TWAP period of quotePeriod
    (int24 _arithmeticMeanTick,) = OracleLibrary.consult(camelotPair, quotePeriod);
    // Calculate the quote amount
    uint256 _quoteAmount = OracleLibrary.getQuoteAtTick({
      tick: _arithmeticMeanTick,
      baseAmount: baseAmount,
      baseToken: baseToken,
      quoteToken: quoteToken
    });
    // Process the quote result to 18 decimal quote
    _result = _parseResult(_quoteAmount);
    _validity = true;
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a proper TWAP that provides the average value of a security over a specified time. The time period/windows of the TWAP must be explicitly defined (e.g. 15 minutes, 1 hour, 24 hours) in the contract.

Assessed type

Oracle

c4-pre-sort commented 1 year ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 1 year ago

raymondfam marked the issue as duplicate of #75

c4-judge commented 1 year ago

MiloTruck marked the issue as not a duplicate

c4-judge commented 1 year ago

MiloTruck marked the issue as duplicate of #226

c4-judge commented 1 year ago

MiloTruck marked the issue as unsatisfactory: Invalid