TWAPOracle might register with wrong token order

[code423n4]

Handle

cmichel

Vulnerability details

The TWAPOracle.registerPair function takes in a factory and (token0, token1). The function accepts a _factory argument which means any Uniswap-like factory can be used.

When using the actual Uniswap factory's IUniswapV2Factory(factory).getPair(token0, token1) call, it could be that the token0 and token1 are reversed as it ignores the order.

Meaning, the price0/1CumulativeLast could also be reversed as it matches the internal order. The code however pushes the _pairs assuming that the internal price0CumulativeLast, price1CumulativeLast order matches the order of the function arguments token0, token1.

_pairs.push(
    PairData({
        pair: pairAddr,
        token0: token0,
        token1: token1,
        price0CumulativeLast: price0CumulativeLast,
        price1CumulativeLast: price1CumulativeLast,
        blockTimestampLast: blockTimestampLast,
        price0Average: FixedPoint.uq112x112({_x: 0}),
        price1Average: FixedPoint.uq112x112({_x: 0})
    })
);

Impact

The prices could be inverted which leads to the oracle providing wrong prices.

Recommended Mitigation Steps

It should be checked if Uniswap's internal order matches the order of the token0/1 function arguments. If not, the cumulative prices must be swapped.

// pseudocode
IUniswapV2Pair pair = IUniswapV2Pair(
    IUniswapV2Factory(factory).getPair(token0, token1)
);
pairAddr = address(pair);
price0CumulativeLast = pair.price0CumulativeLast();
price1CumulativeLast = pair.price1CumulativeLast();
(price0CumulativeLast, price1CumulativeLast) = token0 == pair.token0() ? (price0CumulativeLast, price1CumulativeLast) : (price1CumulativeLast, price0CumulativeLast);

The same issue exists in update

[SamSteinGG]

The TWAP oracle module has been completely removed and redesigned from scratch as LBTwap that is subject of the new audit.