sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

R2 - Using different UniV3 pools may lead to extra fees #143

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

R2

medium

Using different UniV3 pools may lead to extra fees

Summary

Using different UniV3 pools may lead to extra fees paid

Vulnerability Detail

UniV3OracleImpl has variable $_pairOverrides to override pairs using while checking price And it's expected that route Token0->Token1 uses the same pool as Token1->Token0 (with the same fee) But in function UniV3OracleImpl._setPairOverride() you override fee only for Token0->Token1 route:

$_pairOverrides[scqp.cToken0][scqp.cToken1] = params_.pairOverride;

Impact

Let's consider the following situation:

Token TEST with high volatility

  1. You override pair TEST->USDT by next params: {fee: 1%, period: HIGH_PERIOR}
  2. While checking UniV3OracleImpl.getQuoteAmounts() for route TEST->USDT you get correct info from desired pool
  3. But if someone requests UniV3OracleImpl.getQuoteAmounts() for route USDT->TEST, it will use another pool with default params: {fee: $defaultFee, period: $defaultPeriod}. And the result may be unpredictably wrong

Result: your users may pay extra fees or use too short interval to check price for super-high volatile token and loss their funds as a result

Code Snippet

https://github.com/sherlock-audit/2023-04-splits/blob/7303cc26205f10ca9111be31f3574d2573df92b1/splits-oracle/src/UniV3OracleImpl.sol#L240

Tool used

Manual Review

Recommendation

    function _setPairOverride(SetPairOverrideParams calldata params_) internal {
        SortedConvertedQuotePair memory scqp = _convertAndSortQuotePair(params_.quotePair);
        $_pairOverrides[scqp.cToken0][scqp.cToken1] = params_.pairOverride;
        $_pairOverrides[scqp.cToken1][scqp.cToken0] = params_.pairOverride;
    }