WiseOracleHub::latestResolverTwap() could result in DOS.

[hats-bug-reporter[bot]]

Github username: @https://github.com/betharavikiran Twitter username: @ravikiranweb3 Submission hash (on-chain): 0xe32d02a005dd5cb6f2bd97508b75f8981400a43137f25c1447520c1275ca5393 Severity: medium

Description: Description\ WiseOracleHub::latestResolverTwap() does not handle ERC20 tokens with more than 18 decimals and will throw error.

ERC20 by default has 18 decimals positions. But, that is not the upper limit. There are many ERC20 token that has more than 18 decimals. Hence, protocol should take precautionary stance to handle cases where decimals can be more that 18 positions.

There are examples in the WiseOracleHub contract where such handling was done properly. But, for latestResolverTwap(), the decimal position handling was not done correctly and hence would lead to reverting of calls.

Refer to the below line where decimals of Eth - decimals of token is done. If the decimals of tokens is greater than 18, the function will revert causing DOS.

10 ** (_decimalsWETH - decimals(_tokenAddress))

Also, not the above lines are in denominator.

function latestResolverTwap(
        address _tokenAddress
    )
        public
        view
        returns (uint256)
    {
        UniTwapPoolInfo memory uniTwapPoolInfoStruct = uniTwapPoolInfo[
            _tokenAddress
        ];

        if (uniTwapPoolInfoStruct.isUniPool == true) {

            return _getTwapPrice(
                _tokenAddress,
                uniTwapPoolInfoStruct.oracle
            )
                / 10 ** (_decimalsWETH - decimals(_tokenAddress));
        }

        return _getTwapDerivatePrice(
            _tokenAddress,
            uniTwapPoolInfoStruct
        )
            / 10 ** (_decimalsWETH - decimals(_tokenAddress));
    }

Attack Scenario\ The call to WiseOracleHub:: latestResolverTwap() for any token that has more than 18 decimals should lead us to this error.

Attachments

1. Proof of Concept (PoC) File latestResolverTwap() call for any token with more than 18 decimals should lead us to this error.

2. Revised Code File (Optional)

Recommendation is to revise the function as below inline with other functions in the contract

function latestResolverTwap(
        address _tokenAddress
    )
        public
        view
        returns (uint256)
    {
        UniTwapPoolInfo memory uniTwapPoolInfoStruct = uniTwapPoolInfo[
            _tokenAddress
        ];

        uint256 tokenDecimals = decimals(_tokenAddress));
        if (uniTwapPoolInfoStruct.isUniPool == true) {
            if(_decimalsETH < tokenDecimals)
            {
               return _getTwapPrice(
                _tokenAddress,
                uniTwapPoolInfoStruct.oracle
            )
                / 10 ** (tokenDecimals - _decimalsETH);
            }
            else{
               return _getTwapPrice(
                _tokenAddress,
                uniTwapPoolInfoStruct.oracle
                )
                / 10 ** (_decimalsETH - tokenDecimals);
            }
        }
         if(_decimalsETH < tokenDecimals){
          return _getTwapDerivatePrice(
                _tokenAddress,
                uniTwapPoolInfoStruct
            )
                / 10 ** (tokenDecimals - _decimalsETH);
         }
         else{
           return _getTwapDerivatePrice(
            _tokenAddress,
            uniTwapPoolInfoStruct
        )
            / 10 ** (_decimalsETH - tokenDecimals);
       }
    }

[vm06007]

Hello @betharavikiran, as you are saying There are many ERC20 token that has more than 18 decimals could you please provide just few examples of popular tokens in existence with decent liquidity that a protocol would even consider adding to their list? (don't need many, just few)

Also I would like to point that our protocol is intentional not to support tokens with more than 18 decimals when dealing with TWAP, only for regular wiseOracleHub. As TWAP can only be setup for tokens that are with high enough liquidity on UniswapV3. If you can find any token with decent liquidity on UNIV3 that is considered to be added into TWAP check and has more than 18 decimals. But for time being its not intended. In fact not even all 18 decimals tokens would be considered to be added to TWAP checks, only if there's really big liquidity to consider combining this with a price feed.

[betharavikiran]

Then why did you put the logic for handling Tokens with more than 18 decimals in the same contract in other functions.

Pls check that. You can speak both sides, well your protocol, you can do what you like.

Thanks

On Sun, 11 Feb, 2024, 6:55 pm Vitalik Marincenko, @.***> wrote:

Hello @betharavikiran https://github.com/betharavikiran, as you are saying There are many ERC20 token that has more than 18 decimals could you please provide just few examples of popular tokens in existence with decent liquidity that a protocol would even consider adding to their list? (don't need many, just few)

Also I would like to point that our protocol is intentional not to support tokens with more than 18 decimals when dealing with TWAP, only for regular wiseOracleHub. As TWAP can only be setup for tokens that are with high enough liquidity on UniswapV3. If you can find any token with decent liquidity on UNIV3 that is considered to be added into TWAP check and has more than 18 decimals. But for time being its not intended. In fact not even all 18 decimals tokens would be considered to be added to TWAP checks, only if there's really big liquidity to consider combining this with a price feed.

— Reply to this email directly, view it on GitHub https://github.com/hats-finance/Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573/issues/25#issuecomment-1937754252, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACTGZVNE6TEOUB7HJLF27MTYTDBE3AVCNFSM6AAAAABDDPMFJ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZXG42TIMRVGI . You are receiving this because you were mentioned.Message ID: <hats-finance/Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573/issues/25/1937754252 @github.com>

[vm06007]

Then why did you put the logic for handling Tokens with more than 18 decimals in the same contract in other functions. Pls check that. You can speak both sides, well your protocol, you can do what you like. Thanks … On Sun, 11 Feb, 2024, 6:55 pm Vitalik Marincenko, @.***> wrote: Hello @betharavikiran https://github.com/betharavikiran, as you are saying There are many ERC20 token that has more than 18 decimals could you please provide just few examples of popular tokens in existence with decent liquidity that a protocol would even consider adding to their list? (don't need many, just few) Also I would like to point that our protocol is intentional not to support tokens with more than 18 decimals when dealing with TWAP, only for regular wiseOracleHub. As TWAP can only be setup for tokens that are with high enough liquidity on UniswapV3. If you can find any token with decent liquidity on UNIV3 that is considered to be added into TWAP check and has more than 18 decimals. But for time being its not intended. In fact not even all 18 decimals tokens would be considered to be added to TWAP checks, only if there's really big liquidity to consider combining this with a price feed. — Reply to this email directly, view it on GitHub <#25 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACTGZVNE6TEOUB7HJLF27MTYTDBE3AVCNFSM6AAAAABDDPMFJ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZXG42TIMRVGI . You are receiving this because you were mentioned.Message ID: <hats-finance/Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573/issues/25/1937754252 @github.com>

Like stated above, for general purpose can be handled, but absolutely not a security concern. For TWAP this is not intended In fact can probably also remove and make it 18 decimals or less as standard everywhere else. I've asked you few questions about providing token example but did not receive any answers.