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

2 stars 2 forks source link

Malicious user can deposit low valued collateral LST in order to withdraw higher valued collateral LST #298

Closed c4-bot-6 closed 2 months ago

c4-bot-6 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L491-L616 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206-L262 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L279-L311

Vulnerability details

Background

The protocol allows native ETH and different LSTs (liquid staking tokens) to be deposited as collateral in exchange of ezETH. In the current status, it accepts stETH and wBETH but it can accept other LSTs as well if configured.

As you can see below, this is a deposit function for LSTs, in line 507, it retrieved the current value of the collateral in which the minted ezETH will depend on.

File: RestakeManager.sol
491:     function deposit(
492:         IERC20 _collateralToken,
493:         uint256 _amount,
494:         uint256 _referralId
495:     ) public nonReentrant notPaused {

~continue
505: 
506:         // Get the value of the collateral token being deposited
507:         uint256 collateralTokenValue = renzoOracle.lookupTokenValue(_collateralToken, _amount);
508: 

~continue
563: 
564:         // Calculate how much ezETH to mint
565:         uint256 ezETHToMint = renzoOracle.calculateMintAmount(
566:             totalTVL,
567:             collateralTokenValue,
568:             ezETH.totalSupply()
569:         );
570: 
571:         // Mint the ezETH
572:         ezETH.mint(msg.sender, ezETHToMint);
573: 
574:         // Emit the deposit event
575:         emit Deposit(msg.sender, _collateralToken, _amount, ezETHToMint, _referralId);
576:     }

See below. This is a deposit function for native ETH. In line 607, it just retrieved the msg.value to get the minted ezETH.

File: RestakeManager.sol
592:     function depositETH(uint256 _referralId) public payable nonReentrant notPaused {
593:         // Get the total TVL
594:         (, , uint256 totalTVL) = calculateTVLs();
595: 
596:         // Enforce TVL limit if set
597:         if (maxDepositTVL != 0 && totalTVL + msg.value > maxDepositTVL) {
598:             revert MaxTVLReached();
599:         }
600: 
601:         // Deposit the remaining ETH into the DepositQueue
602:         depositQueue.depositETHFromProtocol{ value: msg.value }();
603: 
604:         // Calculate how much ezETH to mint
605:         uint256 ezETHToMint = renzoOracle.calculateMintAmount(
606:             totalTVL,
607:             msg.value,
608:             ezETH.totalSupply()
609:         );
610: 
611:         // Mint the ezETH
612:         ezETH.mint(msg.sender, ezETHToMint);
613: 
614:         // Emit the deposit event
615:         emit Deposit(msg.sender, IERC20(address(0x0)), msg.value, ezETHToMint, _referralId);
616:     }

Issue

In the withdrawal function, the user has a choice to withdraw what type of collateral they want to withdraw. In this scenario, there could be an arbitrage opportunity in which a malicious user can deposit lower valued LST and withdraw higher valued LST. As per current market condition, not all LSTs are equal in terms of pricing, so there is a high chance this price difference can be use as arbitrage opportunity to drain the high valued assets from the protocol to the malicious user.

A good example of two LSTs that can have price difference is between wbETH and stETH. wbETH price is always higher than stETH price in terms of USD. See below for the market price links.

wbETH - https://coinmarketcap.com/currencies/wrapped-beacon-eth/ stETH - https://coinmarketcap.com/currencies/steth/#Chart

As of date of writing May 8 wbETH price is $3,145 while stETH price is $3,029

Please see below the withdraw function (line 206, parameter asset out) in which the user is free to withdraw whatever type of collateral assets.

File: WithdrawQueue.sol
201:     /**
202:      * @notice  Creates a withdraw request for user
203:      * @param   _amount  amount of ezETH to withdraw
204:      * @param   _assetOut  output token to receive on claim
205:      */
206:     function withdraw(uint256 _amount, address _assetOut) external nonReentrant { 
207:         // check for 0 values
208:         if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput();
209: 
210:         // check if provided assetOut is supported
211:         if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset();
212: 

Impact

Economic losses to depositors of higher valued LST collaterals

Proof of Concept

This could be the scenario of the exploit.

  1. User A deposited a higher valued LST of 1 wbETH amounting 3,100 usd.
  2. User B sees the transaction of User A and immediately backruns it by depositing a lower valued LST of 1 stETH amounting 3,000 usd.
  3. User A receives a minted 1.06 ezETH in exchange of deposited 1 wbETH.
  4. User B receives a minted 1.02 ezETH in exchange of deposited 1 stETH.
  5. User B seeing the price difference, immediately withdraws 1.02 ezETH but the actual collateral token to be withdraw is in the form of wbETH.
  6. After the cooldown period (exchange rate did no change), User B able to claim 1.02 wbETH amounting 3,162 usd. Let's say there are other user who deposited 0.02 wbETH so user B able to claim the whole 1.02 wbETH. He got a profit of 162 usd (3,162 minus original deposit cost 3,000).
  7. User A also decided to withdraw his asset but the only remaining LST is 1 stETH. If he decided to withdraw it, he will lose 100 usd (1 stETH is equivalent to 3,000 usd versus original deposit of wbETH amounting 3,100 usd).

Tools Used

Manual Review

Recommended Mitigation Steps

The protocol should only allow withdrawal of collateral originally deposited by the user.

Assessed type

Other

raymondfam commented 2 months ago

@howlbot accept

raymondfam commented 2 months ago

Same root cause as in #108.