Skip to content

Latest commit

 

History

History
156 lines (116 loc) · 5.42 KB

032.md

File metadata and controls

156 lines (116 loc) · 5.42 KB

Bitter Rouge Alpaca

High

Incorrect price conversion

Summary

The OracleUtils ethToToken()/ethToTokenRoundUp() incorrectly returns an output tokenAmount in a ETH decimal instead of the tokenDecimals. So if token==BTC, it returns btcAmount in 18 decimal which is incorrect, and could do severe damage to the protocol.

Root Cause

The nuAssetManager.sol uses ethToToken/ethToTokenRoundUp() function to convert input ethAmount to tokenAmount. To find the root cause, If we compare tokenToEth() and ethToToken() ending calculation of decimal conversion for "X / ETH"

It should be as,

ethAmount = (tokenAmount * tokenPrice * ethDecimal) / (feedDecimal * tokenDecimal) // ------------------(1) 
tokenAmount = (ethAmount * feedDecimal * tokenDecimal) / (tokenPrice * ethDecimal) // ------------------(2)

where the tokenToEth() done as above (1), however, its incorrectly done for the ethToToken(), https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/libraries/OracleUtils.sol#L90-L99

        } else {
            tokenAmount = FullMath.mulDiv(
                _ethAmount,
                10 ** AggregatorV3Interface(_pricefeed).decimals(),
                uint256(price)
            );
        }

        tokenAmount = tokenAmount * 10 ** (18 - _decimals); // ------------------(3)
    }

Since inputToken here is ETH, the _decimals==18. From (3), we can deduce the tokenAmount returned in the same decimal as _ethAmount. If the outputToken is BTC or high value token with decimal lower than 18, the protocol will face severe damage due to high value leak.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Fund loss due to incorrect pricing

PoC

Create a test file, under contracts/Test/utils/BuggyPricing.t.sol and run forge test --via-ir --mt testBuggyPriceConversion --fork-block-number 21413043 --fork-url https://eth-mainnet.g.alchemy.com/v2/cmnVRZ7q4nn5moCWCvUbnRANwtPM1VOs -vv

// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.20;

import "forge-std/Test.sol"; 
import "../../libraries/OracleUtils.sol";

contract BuggyPricingTest is Test , OracleUtils { 
    OracleUtils public util;

    address public constant BTC_ETH = 0xdeb288F737066589598e9214E782fa5A8eD689e8; 
    address public constant ETH_BTC = 0xAc559F25B1619171CbC396a50854A3240b6A4e99; 

    constructor() OracleUtils(address(0)) {}

    function setUp() public { 
        util = new OracleUtils(address(0)); 
    }

    function testBuggyPriceConversion() public { 
        uint256 btcAmount = 1e8; 
        uint256 btcDecimal = 8; 
        uint256 ethDecimal = 18; 
        address priceFeed = BTC_ETH; 

        uint256 ethAmount = tokenToEth(btcAmount, priceFeed, uint128(block.timestamp), btcDecimal);
        uint256 incorrectBtcAmount = ethToToken(ethAmount, priceFeed, uint128(block.timestamp), ethDecimal); // as per [here](https://github.com/sherlock-audit/2024-12-numa-audit/blob/ae1d7781efb4cb2c3a40c642887ddadeecabb97d/Numa/contracts/nuAssets/nuAssetManager.sol#L202)
        uint256 correctedBtcAmount = ethToTokenCorrected(ethAmount, priceFeed, uint128(block.timestamp), btcDecimal); // should be tokenDecimal instead

        console.log("actual btcAmount: ", incorrectBtcAmount); 
        console.log("expected btcAmount: ", correctedBtcAmount); 
    }

    function ethToTokenCorrected(
        uint256 _ethAmount,
        address _pricefeed,
        uint128 _chainlink_heartbeat,
        uint256 _decimals // should be token decimal not ETH 
    ) public view checkSequencerActive returns (uint256 tokenAmount) {
        (
            uint80 roundID,
            int256 price,
            ,
            uint256 timeStamp,
            uint80 answeredInRound
        ) = AggregatorV3Interface(_pricefeed).latestRoundData();

        // heartbeat check
        require(
            timeStamp >= block.timestamp - _chainlink_heartbeat,
            "Stale pricefeed"
        );

        // minAnswer/maxAnswer check
        IChainlinkAggregator aggregator = IChainlinkAggregator(
            IChainlinkPriceFeed(_pricefeed).aggregator()
        );
        require(
            ((price > int256(aggregator.minAnswer())) &&
                (price < int256(aggregator.maxAnswer()))),
            "min/max reached"
        );

        require(answeredInRound >= roundID, "Answer given before round");

        //if ETH is on the left side of the fraction in the price feed
        if (ethLeftSide(_pricefeed)) {
            tokenAmount = FullMath.mulDiv(
                _ethAmount,
                uint256(price),
                10 ** AggregatorV3Interface(_pricefeed).decimals()
            );
        } else {
            tokenAmount = FullMath.mulDiv(
                _ethAmount,
                10 ** AggregatorV3Interface(_pricefeed).decimals(),
                uint256(price)
            );
        }

        // audit fix
        tokenAmount = tokenAmount  / 10 ** (18 - _decimals); //  note that the _decimals here represent tokenDecimals  
    }
}

Heres the SS output, Screenshot from 2024-12-16 15-01-47

Mitigation

In ethToToken() and ethToTokenRoundUp(), modify below line

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