c4-bot-5 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L2/xRenzoDeposit.sol#L245 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L2/xRenzoDeposit.sol#L289-L301 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol#L50-L57

Vulnerability details

Impact

The Bridge/L2/Oracle/RenzoOracleL2.sol#getMintRate incorreclty converts the negative reported prices from the oracle to a very large value for getting the mint rate which will effectively DOS the system functionalities depending on the price rate of Minting from this contract.


  function getMintRate() public view returns (uint256, uint256) {
        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
        // scale the price to have 18 decimals
        uint256 _scaledPrice = (uint256(price)) * 10 ** (18 - oracle.decimals());
        if (_scaledPrice < 1 ether) revert InvalidOraclePrice();
        return (_scaledPrice, timestamp);
    }

For example xRenzoDeposit#deposit method fetches price from this function , and with no validation of price amount, it calculates the eth amount to be returned


  function _deposit(
        uint256 _amountIn,
        uint256 _minOut,
        uint256 _deadline
    ) internal returns (uint256) {
         //snip

        // Fetch price and timestamp of ezETH from the configured price feed
        (uint256 _lastPrice, uint256 _lastPriceTimestamp) = getMintRate();

        // snip
        uint256 xezETHAmount = (1e18 * amountOut) / _lastPrice;

        // Check that the user will get the minimum amount of xezETH
        if (xezETHAmount < _minOut) {
            revert InsufficientOutputAmount();
        }
        //snip

// Mint xezETH to the user
        IXERC20(address(xezETH)).mint(msg.sender, xezETHAmount);

        // Emit the event and return amount minted
        emit Deposit(msg.sender, _amountIn, xezETHAmount);
        return xezETHAmount;
    }

The getMintRate of RenzoDeposit calls the RenzoOracleL2.getMintRate() method.

When negative prices are returned by the oracle , they are incorrectly converted to their UINT version . Which is effectively nothing but uint256.max - (absolute of value).

So for reported price of -1 , returned value will be unit(-1)=115792089237316195423570985008687907853269984665640564039457584007913129639935

uint(-2)=115792089237316195423570985008687907853269984665640564039457584007913129639934

so on and so forth.

Now the getMintRate method calculates the scaled value like this

 uint256 _scaledPrice = (uint256(price)) * 10 ** (18 - oracle.decimals());
        if (_scaledPrice < 1 ether) revert InvalidOraclePrice();

which will cause overflow, revert and cause denial of service for the deposit functionality when the returned prices are negative being a huge number when multiplied by 10^18 ish like number will be out of range for uint256 to hold.

The system will be in DOSed state because there is no fallback oracle to use for example pyth.

So , yeah , that happens .

Likelihood is Low because oracle address can be updated Impact is high because system will not be available until the owner investigates and updates the oracle address If either it is deprecated or just malfunctioning

so marking it as a medium

Proof of Concept

Here's the simple code I've used to check the validity of the issue

 // uint(-2900_00000000) = 115792089237316195423570985008687907853269984665640564039457584007623129639936
    function giveuint(int a)public pure returns(uint) {
        return uint(a);
    }

Tools Used

Remix IDE , Manual review

Recommended Mitigation Steps

Add some kind of fallback oracle to ensure system stability in the events of an underlying oracle fetching wrong prices.

I believe this needs the protocol team to re-consider their Incident Response & Business continuity plans

Assessed type

Oracle

raymondfam commented 4 months ago

@howlbot reject