029.md

Orbiting Sangria Porpoise

Medium

Multiplication instead of division causes incorrect decimal precision conversion in ethToToken() and ethToTokenRoundUp()

Description

Functions ethToToken() and ethToTokenRoundUp() modify the decimal precision between token and ETH by using the following logic:

        tokenAmount = tokenAmount * 10 ** (18 - _decimals);

This is incorrect and division should be used instead of multiplication:

-       tokenAmount = tokenAmount * 10 ** (18 - _decimals);
+       tokenAmount = tokenAmount / 10 ** (18 - _decimals);

Example:

1. We want to convert 1 ETH to USDC using a price of $2000 USD/ETH. Starting values:

    _ethAmount = 1 ETH = 1e18 
    price = $2000 * 1e8 (Chainlink price feed uses 8 decimals)
    _decimals = 6 (USDC decimals)

2. This gives us:

    tokenAmount = FullMath.mulDiv(
        _ethAmount,
        uint256(price), 
        10 ** AggregatorV3Interface(_pricefeed).decimals()
    )   // because `ethLeftSide(_pricefeed) = TRUE`
    
    => 
    tokenAmount = (1e18 * 2000e8) / 1e8
                = 2000e18

3. The returned result ought to be 2000e6 and hence the final step should be:

    tokenAmount = tokenAmount / 10 ** (18 - _decimals)

=>  tokenAmount = tokenAmount / 10 ** (18 - 6)

=>  tokenAmount = 2000e18 / 10 ** 12 = 2000e6

4. However the current logic returns an astronomically high value (10 ** 24 times the correct value):

    tokenAmount = tokenAmount * 10 ** (18 - _decimals)

=>  tokenAmount = tokenAmount * 10 ** (18 - 6)

=>  tokenAmount = 2000e18 * 10 ** 12 = 2000e30

Impact

These functions are only called from inside nuAssetManager.sol through functions ethToNuAsset() and ethToNuAssetRoundUp() which hard-code the _decimals to 18.

The protocol it seems considers that nuAssets will always have a 18-decimal precision. If however this changes in the future and a nuAsset with less than 18 decimals is supported, the impact will be high.

    function ethToNuAsset(
        address _nuAsset,
        uint256 _amount
    ) public view returns (uint256 EthValue) {
        require(contains(_nuAsset), "bad nuAsset");
        nuAssetInfo memory info = getNuAssetInfo(_nuAsset);
        (address priceFeed, uint128 heartbeat) = (info.feed, info.heartbeat);
@-->    return ethToToken(_amount, priceFeed, heartbeat, 18);  // @audit : This should ideally be `return ethToToken(_amount, priceFeed, heartbeat, IERC20Metadata(_nuAsset).decimals());`
    }

    function ethToNuAssetRoundUp(
        address _nuAsset,
        uint256 _amount
    ) public view returns (uint256 EthValue) {
        require(contains(_nuAsset), "bad nuAsset");
        nuAssetInfo memory info = getNuAssetInfo(_nuAsset);
        (address priceFeed, uint128 heartbeat) = (info.feed, info.heartbeat);
@-->    return ethToTokenRoundUp(_amount, priceFeed, heartbeat, 18);  // @audit : This should ideally be `return ethToToken(_amount, priceFeed, heartbeat, IERC20Metadata(_nuAsset).decimals());`
    }

Mitigation

-       tokenAmount = tokenAmount * 10 ** (18 - _decimals);
+       tokenAmount = tokenAmount / 10 ** (18 - _decimals);

and also

    function ethToNuAsset(
        address _nuAsset,
        uint256 _amount
    ) public view returns (uint256 EthValue) {
        require(contains(_nuAsset), "bad nuAsset");
        nuAssetInfo memory info = getNuAssetInfo(_nuAsset);
        (address priceFeed, uint128 heartbeat) = (info.feed, info.heartbeat);
-       return ethToToken(_amount, priceFeed, heartbeat, 18);
+       return ethToToken(_amount, priceFeed, heartbeat, IERC20Metadata(_nuAsset).decimals());
    }

    function ethToNuAssetRoundUp(
        address _nuAsset,
        uint256 _amount
    ) public view returns (uint256 EthValue) {
        require(contains(_nuAsset), "bad nuAsset");
        nuAssetInfo memory info = getNuAssetInfo(_nuAsset);
        (address priceFeed, uint128 heartbeat) = (info.feed, info.heartbeat);
-       return ethToToken(_amount, priceFeed, heartbeat, 18);
+       return ethToToken(_amount, priceFeed, heartbeat, IERC20Metadata(_nuAsset).decimals());
    }