sherlock-audit / 2023-11-olympus-judging

9 stars 8 forks source link

Irissme - UniswapV3OracleHelper Contract: Lack of Token Address Validation in getTWAPRatio Function in Oracle.sol #164

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

Irissme

high

UniswapV3OracleHelper Contract: Lack of Token Address Validation in getTWAPRatio Function in Oracle.sol

Summary

The UniswapV3OracleHelper contract lacks proper verification of token addresses in the getTWAPRatio function. Failure to validate token addresses may pose a security risk if incorrect or malicious addresses are passed to the function.

Vulnerability Detail

The vulnerability lies in the getTWAPRatio function of the UniswapV3OracleHelper contract, where there is a lack of validation for the correctness of the token addresses token0 and token1.

Impact

If incorrect or malicious token addresses are provided to the getTWAPRatio function, it may lead to unpredictable behavior or potential security vulnerabilities, compromising the reliability and security of the UniswapV3OracleHelper contract.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/libraries/UniswapV3/Oracle.sol#L140-L158 Oracle.sol

// ... (previous code)

/// @notice Returns the ratio of token1 to token0 based on the TWAP
///
/// @param pool_ The Uniswap V3 pool
/// @param period_ The period of the TWAP in seconds
/// @param token0_ The `token0` of the pool
/// @param token1_ The `token1` of the pool
/// @param token0Decimals_ The decimals of `token0_`
/// @return The ratio of token1 to token0 in the scale of token1 decimals
function getTWAPRatio(
    address pool_,
    uint32 period_,
    address token0_,
    address token1_,
    uint8 token0Decimals_
) public view returns (uint256) {
    int56 timeWeightedTick = getTimeWeightedTick(pool_, period_);

    // Quantity of token1 for 1 unit of token0 at the time-weighted tick
    // Scale: token1 decimals
    uint256 baseInQuote = OracleLibrary.getQuoteAtTick(
        int24(timeWeightedTick),
        uint128(10 ** token0Decimals_), // 1 unit of token0
        token0_,
        token1_
    );

    return baseInQuote;
}

// ... (remaining code)

Tool used

Manual Review

Recommendation

It is recommended to implement proper validation checks for the correctness of the provided token addresses within the getTWAPRatio function. The validation can include checks for valid Ethereum addresses and whether the addresses correspond to ERC20 tokens.

Here's an example of how the validation checks can be implemented:

require(token0_ != address(0), "Invalid token0 address");
require(token1_ != address(0), "Invalid token1 address");
require(isContract(token0_), "token0 is not a valid contract address");
require(isContract(token1_), "token1 is not a valid contract address");
nevillehuang commented 10 months ago

Invalid, this is speculating on wrong input utilization of the getTWAPRatio. Additionally, zero address checks are not valid based on sherlock rules.

  1. Zero address checks: Check to make sure input values are not zero addresses.