Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RestakeManager susceptible to an inflation attack #60

Closed
howlbot-integration bot opened this issue May 9, 2024 · 1 comment
Closed

RestakeManager susceptible to an inflation attack #60

howlbot-integration bot opened this issue May 9, 2024 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-35 edited-by-warden 🤖_13_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@howlbot-integration
Copy link

Lines of code

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

Vulnerability details

Impact

First depositor can inflate the exchange rate of ezETH token such that any user who deposits afterwards receives less ezETH tokens for their deposits than they should. More info here, check example 2.

Proof of Concept

RestakeManager accepts ETH and ERC20 both as a collateral for minting ezETH token. Both could be used to perform this attack, however for simplicity, I'm going to show it for ETH deposits.

Let's have a look at the formula that is used to calculate the ezETHToMint,
https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L605-L609

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

        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;

The mintAmount is determined by subtracting _existingEzETHSupply from the newEzETHSupply, since those value already calculated above, we can simplified it futher for better view.

	// looks familiar
        uint256 mintAmount = (_newValueAdded * _existingEzETHSupply) / _currentValueInProtocol; 

the _existingEzETHSupply and _currentValueInProtocol(e.g. tvl) intiallialy are 0,

To perform an attack;

        if (_currentValueInProtocol == 0 || _existingEzETHSupply == 0) {
            return _newValueAdded; // value is priced in base units, so divide by scale factor
        }
  • another depositETH() txn sent by a legimate user with 2 ETH of assets. Attacker saw it and try to frontrun it by a depositQueue.forwardFullWithdrawalETH(1 ether) call,

Note here, we cannot directly sent 1 ETH to depositQueue, since receive() will triggered and charge fee on the attacker sent amount.

The forwardFullWithdrawalETH() function here doing same as direct deposit will, also this call fill the withdraw buffer if required(means sending required fraction to withdrawQueue), but that won't be a problem, we'll see ahead.

  • For victim during it deposits,

_newValueAdded = 2 ETH,
_existingEzETHSupply = 1 WEI,
_currentValueInProtocol = totalTVL

totalTVL is retrieved from calculateTVLs() function during the deposits, it can seen below that the totalTVL includes eth balance from depositQueue and withdrawQueue(which possibly filled during forwardFullWithdrawalETH call above) contract as well,
https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L351-L356

        // Get the value of native ETH held in the deposit queue and add it to the total TVL
        totalTVL += address(depositQueue).balance;

        // Add native ETH help in withdraw Queue and totalWithdrawalQueueValue to totalTVL
        totalTVL += (address(withdrawQueue).balance + totalWithdrawalQueueValue);

So the totalTVL = 1 wei + 1 ETH from attacker

Due to rounding, the victim gets (2 ether * 1) / (1 ether + 1) == 1 shares which is same amount as the attacker. Attacker manage to steal (3 ether / 2) - 1 ether == 0.5 ether worth of extra shares from the victim.

        uint256 mintAmount = (_newValueAdded * _existingEzETHSupply) / _currentValueInProtocol; 

Tools Used

Manual review

Recommended Mitigation Steps

There are few mitigation strategies shared here, which could be implemented by Renzo.

Assessed type

ERC4626

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_13_group AI based duplicate group recommendation bug Something isn't working edited-by-warden sufficient quality report This report is of sufficient quality labels May 9, 2024
howlbot-integration bot added a commit that referenced this issue May 9, 2024
@c4-judge
Copy link
Contributor

alcueca marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-35 edited-by-warden 🤖_13_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants