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

2 stars 2 forks source link

Less ezETH tokens inside the RestakeManager contract are minted due to the precision loss inside the `calculateMintAmount()` function #102

Closed c4-bot-4 closed 5 months ago

c4-bot-4 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Oracle/RenzoOracle.sol#L123-L149

Vulnerability details

Impact

Solidity rounds down the result of an integer division, and because of that, it is always recommended to multiply before dividing to avoid that precision loss. In the case of a prior division over multiplication, the final result may face serious precision loss as the first answer would face truncated precision and then multiplied to another integer.

The problem arises in the RenzoOracle's calculateMintAmount() part. This function is responsible for calculating the mint amount corresponding to the given amount of current protocol value, newly added value, and the supply of ezETH.

If we look deeply at this function, we can see how the mentioned computation is being performed:

    function calculateMintAmount(
        uint256 _currentValueInProtocol,
        uint256 _newValueAdded,
        uint256 _existingEzETHSupply
    ) external pure returns (uint256) {
        // For first mint, just return the new value added.
        // Checking both current value and existing supply to guard against gaming the initial mint
        if (_currentValueInProtocol == 0 || _existingEzETHSupply == 0) {
            return _newValueAdded; // value is priced in base units, so divide by scale factor
        }

        // Calculate the percentage of value after the deposit
        uint256 inflationPercentaage = (SCALE_FACTOR * _newValueAdded) /
            (_currentValueInProtocol + _newValueAdded);

        // Calculate the new supply
        uint256 newEzETHSupply = (_existingEzETHSupply * SCALE_FACTOR) /
            (SCALE_FACTOR - inflationPercentaage);

        // Subtract the old supply from the new supply to get the amount to mint
        uint256 mintAmount = newEzETHSupply - _existingEzETHSupply;

        // Sanity check
        if (mintAmount == 0) revert InvalidTokenAmount();

        return mintAmount;
    }

As it is illustrated, we deep dive to check the mathematics behind it. (For simplicity, the SCALE_FACTOR, _newValueAdded, _currentValueInProtocol, and _existingEzETHSupply are considered as $10^{18}$, $v{new}$, $v{protocol}$, and $S_e$).

inflated percentage formula: ($inflated$)

$$ inflated = \frac{10^{18} \times v{new}}{v{protocol} + v_{new}} $$

and, new ezETH supply: ($S_{e, new}$)

$$ S_{e, new} = \frac{10^{18} \times S_e}{10^{18} - inflated} $$

Finally, the mintAmount is calculated as the subtraction of the new and previous total supplies of ezETH:

$$ mintAmounts = S_{e, new} - S_e $$

If we want to simplify the above-mentioned relations to increase the accuracy and also decrease the gas consumption, we can have:

$$ = S_{e, new} - S_e = \LARGE{\frac{10^{18} \times Se}{10^{18} - \Large{\frac{10^{18} \times v{new}}{v{protocol} + v{new}}}}} - \large{S_e} $$

$$ = \LARGE{\frac{10^{18} \times Se \times (v{protocol} + v{new})}{10^{18} \times (v{protocol} + v{new}) - {10^{18} \times v{new}}}} - \large{S_e} $$

$$ = \LARGE{(\frac{10^{18} \times (v{protocol} + v{new})}{10^{18} \times v_{protocol}} - 1)} \large{\times S_e} $$

$$ = \LARGE{\frac{10^{18} \times Se \times v{new}}{10^{18} \times v_{protocol}}} $$

we can see in the actual implementation, there is a hidden division before multiplication in the calculation of the inflationPercentaage, and also newEzETHSupply that rounds down the whole expression. This is bad as the precision loss can be significant, which leads to the low mint amounts to be calculated.

At the Proof of Concept part, we can check this behavior precisely.

Proof of Concept

You can run this code to see the difference between the results:

```Solidity // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.17; import "forge-std/Test.sol"; contract DifferenceTest is Test { uint256 public constant SCALE_FACTOR = 1e18; error InvalidTokenAmount(); function setUp() public { } function calculateMintAmount_Actual( uint256 _currentValueInProtocol, uint256 _newValueAdded, uint256 _existingEzETHSupply ) public pure returns (uint256 mintAmount) { uint256 inflationPercentaage = (SCALE_FACTOR * _newValueAdded) / (_currentValueInProtocol + _newValueAdded); // Calculate the new supply uint256 newEzETHSupply = (_existingEzETHSupply * SCALE_FACTOR) / (SCALE_FACTOR - inflationPercentaage); // Subtract the old supply from the new supply to get the amount to mint mintAmount = newEzETHSupply - _existingEzETHSupply; // Sanity check if (mintAmount == 0) revert InvalidTokenAmount(); return mintAmount; } function calculateMintAmount_Accurate( uint256 _currentValueInProtocol, uint256 _newValueAdded, uint256 _existingEzETHSupply ) public pure returns (uint256 mintAmount) { mintAmount = ((SCALE_FACTOR * _existingEzETHSupply * _newValueAdded) / (SCALE_FACTOR * _currentValueInProtocol)); } function test_diff() public { uint _existingEzETHSupply = 7.3654126549e24; uint _newValueAdded = 5.2315648123541e17; uint _currentValueInProtocol = 1.25314545412354e24; uint actualmintAmount = calculateMintAmount_Actual(_currentValueInProtocol, _newValueAdded, _existingEzETHSupply); uint accuratemintAmount = calculateMintAmount_Accurate(_currentValueInProtocol, _newValueAdded, _existingEzETHSupply); console.log("actual inflated supply is: ", actualmintAmount); console.log("accurate inflated supply is: ", accuratemintAmount); assertFalse(actualmintAmount == accuratemintAmount); } } ```

The result would be:

(for these sample but real variables:

_existingEzETHSupply = 7.3654126549e24,

_newValueAdded = 5.2315648123541e17,

_currentValueInProtocol = 1.25314545412354e24)


     Current Implementation of mint amount : 307487319584_1975152
     Accurate Implementation of mint amount: 307487319584_8798383

Thus, we can see that the actual implementation mints less ezETH tokens than the precise method.

Tools Used

Manual Review Fuzz test

Recommended Mitigation Steps

Consider modifying the mint amount calculation to prevent such precision loss. This modification also reduces the intermediary variables and decreases the gas cost (from 2561 to 1964 gas consumption which means 597 gas amount is saved)

    function calculateMintAmount(
        uint256 _currentValueInProtocol,
        uint256 _newValueAdded,
        uint256 _existingEzETHSupply
    ) external pure returns (uint256) {
        // For first mint, just return the new value added.
        // Checking both current value and existing supply to guard against gaming the initial mint
        if (_currentValueInProtocol == 0 || _existingEzETHSupply == 0) {
            return _newValueAdded; // value is priced in base units, so divide by scale factor
        }

        // Calculate the percentage of value after the deposit
-        uint256 inflationPercentaage = (SCALE_FACTOR * _newValueAdded) /
-            (_currentValueInProtocol + _newValueAdded);

        // Calculate the new supply
-        uint256 newEzETHSupply = (_existingEzETHSupply * SCALE_FACTOR) /
-            (SCALE_FACTOR - inflationPercentaage);

        // Subtract the old supply from the new supply to get the amount to mint
-        uint256 mintAmount = newEzETHSupply - _existingEzETHSupply;
+        uint256 mintAmount = ((SCALE_FACTOR * _existingEzETHSupply * _newValueAdded) / (SCALE_FACTOR * _currentValueInProtocol));

        // Sanity check
        if (mintAmount == 0) revert InvalidTokenAmount();

        return mintAmount;
    }

Assessed type

Math

DadeKuma commented 5 months ago

@howlbot accept