code-423n4 / 2024-06-renzo-mitigation-findings

0 stars 0 forks source link

M-12 MitigationConfirmed #29

Open c4-bot-4 opened 3 months ago

c4-bot-4 commented 3 months ago

Lines of code

Vulnerability details

See:

Finding Mitigation
M-12: Incorrect exchange rate provided to Balancer pools Pull Request

Navigating to M-12 from the previous contest we can see that there was an issue in regards to the xRenzoDeposit.sol contract, which serves as both the entry point for deposits on Layer 2 (L2) networks and a rate provider for Balancer pools, enabling these pools to determine the exchange rate between xezWETH and WETH tokens.

The crux of the issue is the fact that the getRate() function in xRenzoDeposit returns an outdated and potentially incorrect rate by solely relying on the lastPrice state variable, cause this rate can become stale if updatePrice() or updatePriceByOwner() are not called frequently, and could as well differ from the rate used during the minting of xezETH, which utilizes the most recent data from the oracle and lastPrice. As a result, this discrepancy can lead to mispricing in Balancer pools, incorrect yield calculations, and potential manipulation during pool joins and exits.

Now to mitigate this issue, protocol passed this pull request which sufficiently mitigates the issue, considering the getRate() function has now being updated to return the same rate used during the minting of xezETH by calling getMintRate() instead of directly returning lastPrice, i.e:

    function getRate() external view override returns (uint256) {
-        (uint256 _lastPrice, ) = getMintRate();
+         (uint256 _lastPrice, uint256 _lastPriceTimestamp) = getMintRate();
+         if (block.timestamp > _lastPriceTimestamp + 1 days) {
+             revert OraclePriceExpired();
+         }
         return _lastPrice;
     }

Would be key to note that the previous implementation of the function is the below https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Bridge/L2/xRenzoDeposit.sol#L457-L460:

    function getRate() external view override returns (uint256) {
        return lastPrice;
    }

The diff check however did not explicitly update from the previous implementation, however the pull request is justified for the fix, cause with the current adjustment, protocol ensures that the rate provided to Balancer pools is accurate and consistent with the minting process.

c4-judge commented 3 months ago

alcueca marked the issue as satisfactory