sherlock-audit / 2023-11-olympus-judging

9 stars 8 forks source link

tvdung94 - UniswapV2PoolTokenPrice::getTokenPrice() still proceeds even when lookupToken does not exist #133

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

tvdung94

medium

UniswapV2PoolTokenPrice::getTokenPrice() still proceeds even when lookupToken does not exist

Summary

getTokenPrice() still proceeds even when lookupToken does not exist.

Vulnerability Detail

lookupTokenIndex is assumed to be the other token's if it is not the first one. However, this does not stay true all the time; there is a case where lookup token just simply does not exist in the pool. In this case, the contract should just simply revert instead of continuing to proceed.

        uint256 lookupTokenIndex = type(uint256).max;
        uint256 destinationTokenIndex = type(uint256).max;
        uint256 destinationTokenPrice; // Scale: outputDecimals_
        {
            address token0 = tokens_[0];
            address token1 = tokens_[1];

            if (token0 == address(0)) revert UniswapV2_PoolTokensInvalid(address(pool), 0, token0);
            if (token1 == address(0))
                revert UniswapV2_PoolTokensInvalid(address(pool), 1, tokens_[1]);
            if (lookupToken_ != token0 && lookupToken_ != token1)
                revert UniswapV2_LookupTokenNotFound(address(pool), lookupToken_);

>>>   lookupTokenIndex = (lookupToken_ == token0) ? 0 : 1; //@audit - should not assume that lookup token does exist in the pool
            destinationTokenIndex = 1 - lookupTokenIndex;
            (uint256 destinationTokenPrice_, ) = _PRICE().getPrice(
                tokens_[destinationTokenIndex],
                PRICEv2.Variant.CURRENT
            );
            destinationTokenPrice = destinationTokenPrice_;
        }

Impact

Price returned is not for the lookup token if it does not exist, affecting price accounting

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/UniswapV2PoolTokenPrice.sol#L340-L360

Tool used

Manual Review

Recommendation

Consider check the other token as well. If both tokens in the pool are not the lookup token then revert.

nevillehuang commented 9 months ago

Invalid, if lookUpToken is not token0 or token1, it will revert due to this check here