code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

All sub sequent _deposit() function calls DOS if the ``_scaledPrice `` price goes less than 1ETH #717

Closed c4-bot-5 closed 6 months ago

c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol#L55 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/xRenzoDeposit.sol#L245 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Bridge/L2/xRenzoDeposit.sol#L293

Vulnerability details

Impact

All subsequent _deposit() function calls reverts. Even can't use the lastPrice, lastPriceTimestamp as per current implementations.

Proof of Concept

FILE: 2024-04-renzo/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol

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);
    }
FILE: 2024-04-renzo/contracts/Bridge/L2/xRenzoDeposit.sol

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

 /**
     * @notice Fetch the price of ezETH from configured price feeds
     */
    function getMintRate() public view returns (uint256, uint256) {
        // revert if PriceFeedNotAvailable
        if (receiver == address(0) && address(oracle) == address(0)) revert PriceFeedNotAvailable();
        if (address(oracle) != address(0)) {
            (uint256 oraclePrice, uint256 oracleTimestamp) = oracle.getMintRate();
            return
                oracleTimestamp > lastPriceTimestamp
                    ? (oraclePrice, oracleTimestamp)
                    : (lastPrice, lastPriceTimestamp);
        } else {
            return (lastPrice, lastPriceTimestamp);
        }
    }

Possible situations oracle price down

POC

This creates a potential Denial of Service (DoS) all subsequent _deposit() calls become DOS. Yes its possible to access lastPrice, lastPriceTimestamp for alternative but this is only possible situations where,

If oracle is set always price fetched from oracle itself.

Tools Used

Manual Audit

Recommended Mitigation Steps

Change the code to following way.This will solve the issue

FILE: 2024-04-renzo/contracts/Bridge/L2/Oracle/RenzoOracleL2.sol

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);
    }
FILE: 2024-04-renzo/contracts/Bridge/L2/xRenzoDeposit.sol

function getMintRate() public view returns (uint256, uint256) {
    // Revert if no valid data source is available
    if (receiver == address(0) && address(oracle) == address(0)) {
        revert PriceFeedNotAvailable();
    }

    if (address(oracle) != address(0)) {
        // Fetch current price and timestamp from the oracle
        (uint256 oraclePrice, uint256 oracleTimestamp) = oracle.getMintRate();

-        // Check if the current price is valid
-        if (oraclePrice < 1 ether) {
-            // If the oracle price is below 1 ETH, return the last known good price and timestamp
-           return (lastPrice, lastPriceTimestamp);
-       }

        // Return the most recent valid data, comparing by timestamp
        return oracleTimestamp > lastPriceTimestamp ? (oraclePrice, oracleTimestamp) : (lastPrice, lastPriceTimestamp);
    } else {
        // Fallback to the last known price if the oracle is unavailable
        return (lastPrice, lastPriceTimestamp);
    }
}

Assessed type

Oracle

0xJuancito commented 6 months ago

Invalid.

From the sponsor comment on Discord:

  1. The revert condition in RenzoOracleL2 if (_scaledPrice < 1 ether) revert InvalidOraclePrice();

As, ezETH is a yieldBearing Token which means exchangeRate of ezETH >= 1 ETH. The revert condition is placed to provide a protection of protocol collateral against any wrong oracle feed and restricts user to deposit and mint more ezETH then they are supposed to.

0xJuancito commented 6 months ago

@howlbot reject