code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Using Uniswap TWAP on L2 leads to DoS #163

Closed c4-bot-7 closed 5 months ago

c4-bot-7 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/WiseOracleHub.sol#L143-L168 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/OracleHelper.sol#L163-L171

Vulnerability details

UniswapV3 TWAP oracle suffers from "infinite price" manipulation problem. If the price is driven above (or below) last tick possesing liquidity, it's free to manipulate the price further, up to max or min tick. Depending on the liquidity depth and distribution, leading to significant changes in 30 minutes TWAP period requires inflated price for just a few blocks and may be relatively cheap. There are 2 additional factors increasing feasibility of this attack:

Impact

On-demand prolonged system DoS leading to extracting value from positions that can't be executed on time.

Proof of Concept

To illustrate how the attack impacts the system, I'll go through the code that uses oracle pricing. Whenever price is checked in WiseOracleHub.getTokensInETH(), it calls latestResolver() which verifies allowed difference:

    /**
     * @dev Returns ETH value of a given token
     * amount in order of 1E18 decimal precision.
     */
    function getTokensInETH(
        address _tokenAddress,
        uint256 _tokenAmount
    )
        public
        view
        returns (uint256)
    {
        if (_tokenAddress == WETH_ADDRESS) {
            return _tokenAmount;
        }

        uint8 tokenDecimals = _tokenDecimals[
            _tokenAddress
        ];

        return _decimalsETH < tokenDecimals
            ? _tokenAmount
                * latestResolver(_tokenAddress)
                / 10 ** decimals(_tokenAddress)
                / 10 ** (tokenDecimals - _decimalsETH)
            : _tokenAmount
                * 10 ** (_decimalsETH - tokenDecimals)
                * latestResolver(_tokenAddress)
                / 10 ** decimals(_tokenAddress);
    }
    function latestResolver(
        address _tokenAddress
    )
        public
        view
        returns (uint256)
    {
        if (chainLinkIsDead(_tokenAddress) == true) {
            revert OracleIsDead();
        }

        return _validateAnswer( // @audit Chainlink vs Uniswap price difference is done here
            _tokenAddress
        );
    }

Resolver checks if the price is valid, Chainlink oracle is working and difference between UniswapV3 and Chainlink is acceptable in _validateAnswer() function:

    function _validateAnswer(
        address _tokenAddress
    )
        internal
        view
        returns (uint256)
    {
        // [...]
            if (fetchTwapValue > 0) {

            uint256 relativeDifference = _getRelativeDifference( // @audit it's smaller price devided by the bigger
                answer,
                fetchTwapValue
            );

            _compareDifference(
                relativeDifference
            );
        }

At the end, the difference is compared in _compareDifference() to check if the two prices are not too distant from each other:

    function _compareDifference(
        uint256 _relativeDifference
    )
        internal
        view
    {
        if (_relativeDifference > ALLOWED_DIFFERENCE) { // @audit initial value 10250, common for all tokens
            revert OraclesDeviate();
        }
    }

It's worth mentioning that the allowed price diference (expressed in percentage) is common for all token prices, which might be dangerous for low liquidity assets.

So, to summarize, if the prices deviate by too much, it will revert with OraclesDeviate error. It will actually revert until the outlier block values are past TWAP period, which is hardcoded to 30 minutes in the code.

Tools Used

Manual Review

Recommended Mitigation Steps

While we understand the preventive measures of Wiselending protecting against black swan event on Chainlink oracles, we believe that the Wiselending checks should be more dynamic and adjusted on the underlying asset basis. Allow for more dynamic price deviation percentage per pool / chain, because of ease of TWAP manipulation on L2.

Assessed type

DoS

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as sufficient quality report

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Out of scope