xRenzoDeposit can mint at invalid rates

[c4-bot-3]

Lines of code

https://github.com/Renzo-Protocol/Contracts/blob/7dabe5478ec6dbf084135737bb0d88960e80b63e/contracts/Bridge/L2/xRenzoDeposit.sol#L299

Vulnerability details

RenzoOracleL2 has a check implemented, meant to block xezETH mints on L2 whenever the price goes below 1e18:

File: RenzoOracleL2.sol
50:     function getMintRate() public view returns (uint256, uint256) {
---
55:         if (_scaledPrice < 1 ether) revert InvalidOraclePrice();

This check however can be bypassed in xRenzoDeposit because this contract can also work without an oracle configured and with prices streamed from L1:

File: xRenzoDeposit.sol
289:     function getMintRate() public view returns (uint256, uint256) {
290:         // revert if PriceFeedNotAvailable
291:         if (receiver == address(0) && address(oracle) == address(0)) revert PriceFeedNotAvailable();
292:         if (address(oracle) != address(0)) {
293:             (uint256 oraclePrice, uint256 oracleTimestamp) = oracle.getMintRate();
294:             return
295:                 oracleTimestamp > lastPriceTimestamp
296:                     ? (oraclePrice, oracleTimestamp)
297:                     : (lastPrice, lastPriceTimestamp);
298:         } else {
299:             return (lastPrice, lastPriceTimestamp);
300:         }
301:     }
302: 

It's worth noting that while prices are streamed by an authority in L1, these prices can't be controlled by this authority, and their source (BalancerRateProvider) does not have either a price < 1e18 check:

File: BalancerRateProvider.sol
29:     function getRate() external view returns (uint256) {
30:         // Get the total TVL priced in ETH from restakeManager
31:         (, , uint256 totalTVL) = restakeManager.calculateTVLs();
32: 
33:         // Get the total supply of the ezETH token
34:         uint256 totalSupply = ezETHToken.totalSupply();
35: 
36:         // Sanity check
37:         if (totalSupply == 0 || totalTVL == 0) revert InvalidZeroInput();
38: 
39:         // Return the rate
40:         return (10 ** 18 * totalTVL) / totalSupply;
41:     }

Impact

When operating without an oracle, xRenzoDeposit can mint at prices below 1e18, bypassing the checks implemented in RenzoOracleL2.

Proof of Concept

• Have xRenzoDeposit implemented with oracle == address(0)
• Have some slashing in L1 cause the ezETH price go below that of Ether
• At this point, the protocol will be in two situations, both incorrect:
  • L2 will mint at an incorrect rate (outdated)
  • L2 price will be updated, but this will inevitably allow minting at below 1e18

Tools Used

Code review

Recommended Mitigation Steps

Consider adding a price < 1e18 check also in xRenzoDeposit.getMintRate().

Assessed type

Invalid Validation

[jatinj615]

Protocol did a consistent price check on L2 i.e. if Price reported by Receiver is less than 1 ETH then revert. - https://github.com/Renzo-Protocol/Contracts/pull/96/files#diff-478c8300348d5b84e98f49d7cc8c761b1f45617cdbcb5ed3487dfc563a9aac8cR347. So this would invalidate this as price cannot be < 1 ETH on L2s.

[0xEVom]

Thanks @jatinj615. I believe this wasn't in the mitigation scope, so the finding would remain valid.

[bronzepickaxe]

This issue is out-of-scope. This issue (wrongly) describes the same root issue that has been extensively documented in our issue:

• https://github.com/code-423n4/2024-04-renzo-findings/issues/615 and in the primary finding:
• https://github.com/code-423n4/2024-04-renzo-findings/issues/117

Adding

• if (_scaledPrice < 1 ether) revert InvalidOraclePrice()

to xRenzoDeposit.sol will not do anything because it can still be side-stepped by the L1 CCIP calls, as extensively described in the issues above.

The mitigation provided by the Sponsor is a fix for the issues provided above. Even though this mitigation is not in scope, findings submitted that are the same as findings with a sponsor acknowledged label, are out of scope.

The mitigation provided by the Sponsor being in-scope or out-of-scope is irrelevant since this issue is out-of-scope due to the fact that it's the same issue that has already been labelled as acknowledged by the Sponsor.

Furthermore, this report is factually incorrect. The report claims being able to function without an oracle. This means it won't receive price updates from either L1 or L2, resorting to the admin manually updating the price by calling `updatePriceByOwner():

    /**
     * @notice  Updates the price feed from the Owner account
     * @dev     Sets the last price and timestamp
     * @param   price  price of ezETH to ETH - 18 decimal precision
     */
    function updatePriceByOwner(uint256 price) external onlyOwner {
        return _updatePrice(price, block.timestamp);
    }

This might be hypothetically possible but no project would hire an engineer to manually update the price.

If the report was meant to describe the scenario where the Sponsor is only operating by receiving L1 CCIP calls to update the price feed, then that is correct, this is one of the configurations that is possible.

However, the issue described in this report, being able to side-step the (_scaledPrice < 1 ether) has been already been described in our original issue:

• https://github.com/code-423n4/2024-04-renzo-findings/issues/615

Thanks!

[c4-judge]

alcueca marked the issue as unsatisfactory: Out of scope