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

8 stars 6 forks source link

Incorrect Calculation of Token in Eth Price #279

Closed c4-bot-6 closed 3 months ago

c4-bot-6 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/WiseOracleHub.sol#L159-L167

Vulnerability details

Impact

The WiseOracleHub.getTokensInEth gets the value of given tokens in ETH using a calculation which calculates the value based on the token decimals.

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);
    }

The value of the tokens in ETH is used by very crucial functions in the protocol including WiseOracleHub.getTokensPriceInUSD, FeeManager.getReceivingToken, WiseCore._coreLiquidation, PendlePowerFarmMathLogic.getTotalWeightedCollateralETH and numerous other essential functions. In the case of incorrect value returned by this function, the impact on the entire protocol is far reaching and extremely critical.

Proof of Concept

 return _decimalsETH < tokenDecimals ? 
              _tokenAmount * 
              latestResolver(_tokenAddress) 
              / 10 ** decimals(_tokenAddress)
               / 10 ** (tokenDecimals - _decimalsETH)
            : _tokenAmount
                * 10 ** (_decimalsETH - tokenDecimals)
                * latestResolver(_tokenAddress)
                / 10 ** decimals(_tokenAddress);

Given: _decimalsETH = 18 tokenDecimals = 27 (Assume a token with 27 decimals) tokenAmount = 1 (Assume we want to convert just one token) latestResolver(token) = 1000000000000000000 (This is paired to ETH therefore chainlink feed would return the price in 18 decimals; Also, let us assume that the the resolver returns the price of the token as 1 ether or 1e18 i.e. 1 token = 1 ETH. decimals(token) = 18 (As this is paired with ETH) Since ethDecimals < tokenDecimals in this scenario, the first calculation in the tenary is returned.

uint256 answer = 1 * 1000000000000000000 / 10**18 / 10 ** (27-18)

In this scenario, answer would actually return 0 due to rounding down in solidity. Therefore, in a scenario where we are trying to convert 1 token to Eth whose value is 1 ether, this should return 1 but we get 0.

Tools Used

Manual Review

Recommended Mitigation Steps

  1. The getTokensInEth function calculation must be corrected to return the correct value of the tokens and also if rounding down occurs there must be a provision for that scenario.

Assessed type

Math

c4-pre-sort commented 4 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 4 months ago

Seems to be a massive edge case that is being overblown

c4-pre-sort commented 4 months ago

GalloDaSballo marked the issue as primary issue

vonMangoldt commented 3 months ago

If i undertand correctly this is for a token with greater than 18 decimals and therfor oos since master decides which pools to add and no relevant token is using higher decimals and can in theory always be abstraced down with a wrapper to 18 decimals

vm06007 commented 3 months ago

based on comments this should be invalidated and closed

c4-judge commented 3 months ago

trust1995 marked the issue as unsatisfactory: Invalid