c4-bot-4 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts%2FBridge%2FL2%2FOracle%2FRenzoOracleL2.sol#L55

Vulnerability details

Summary

The idea of setting slippage is to protect the user from getting less tokens than they should due to high volatility and stop them from being exploited.

The RenzoOracleL2.sol contract specially the getMintRate() function using the aggregator v3 to get/call the latestRoundData(). The function should check for the min and max price returned according Chainlink Documentation.

However, in RenzoOracleL2::getMintRate(), the function only checks for the minPrice set at 1 ether but fails to check for the maxPrice.

Proof of Concept

RenzoOracleL2::getMintRate()

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts%2FBridge%2FL2%2FOracle%2FRenzoOracleL2.sol#L55

     * @dev reverts if price is less than 1 Ether
     */
    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);
    }

The getMintRate() pulls the price of ezETH. This determines the number of tokes received upon minting.

Here is the application:

xRenzoDeposit::deposit() It accepts deposit for the user in depositToken and mints xezETH.
function deposit(
uint256 _amountIn,
uint256 _minOut,
uint256 _deadline
) external nonReentrant returns (uint256) {
//...
return _deposit(_amountIn, _minOut, _deadline);
}
The function makes an internal call to _deposit() in its return statement.

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

// Verify the price is not stale
if (block.timestamp > _lastPriceTimestamp + 1 days) {
    revert OraclePriceExpired();
}

// Calculate the amount of xezETH to mint
uint256 xezETHAmount = (1e18 * amountOut) / _lastPrice;

getMintRate() is used here to fetch the price and timestamp of ezETH from the configured price feed. The price retrieved, _lastPrice, is then used in the calculation to determine the amount of xezETH to mint.

The absence of a maximum price check in getMintRate() means that if the price of ezETH fetched is unexpectedly high due to an error or manipulation, the contract will mint a smaller amount of xezETH than it should. This is because the minted amount is inversely proportional to the price:

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

Impact

A higher _lastPrice results in less xezETH minted for the depositor, potentially leading to losses for users if the price is incorrect.

Tools Used

Manual Review

Recommended Mitigation Steps

A circuit breaker should be implemented on the oracle so that when the price edges close to maxPrice it starts reverting, to avoid consuming very high prices.

Define the maxPrice then refactor the check as follows:

        if (_scaledPrice < 1 ether || _scaledPrice > maxPrice) revert InvalidOraclePrice();

Assessed type

Oracle

raymondfam commented 5 months ago

@howlbot reject