Renzo - m_Rassska's results

A protocol that abstracts all staking complexity from the end-user and enables easy collaboration with EigenLayer node operators and a Validated Services (AVSs).

General Information

Platform: Code4rena

Start Date: 30/04/2024

Pot Size: $112,500 USDC

Total HM: 22

Participants: 122

Period: 8 days

Judge: alcueca

Total Solo HM: 1

Id: 372

League: ETH

Renzo

Findings Distribution

Researcher Performance

Rank: 95/122

Findings: 2

Award: $0.04

🌟 Selected for report: 0

🚀 Solo Findings: 0

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L318

Vulnerability details

  • Upon calculating the TVL of the system, the eth balance held by WithdrawQueue should also be accounted, since the shares are not burnt, when the request is submitted. However, due to the loop index mismatch the computation fails, since odLength could be more than tokenLength.

Impact

  • DoS
  • Short term:
          // record token value of withdraw queue
          if (!withdrawQueueTokenBalanceRecorded) { 
              totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
    +-        collateralTokens[j], 
                  collateralTokens[j].balanceOf(withdrawQueue)
              );
          }

Assessed type

Context

#0 - c4-judge

2024-05-16T10:34:43Z

alcueca marked the issue as satisfactory

#1 - c4-judge

2024-05-16T10:39:08Z

alcueca changed the severity to 3 (High Risk)

#2 - c4-judge

2024-05-20T04:26:26Z

alcueca changed the severity to 2 (Med Risk)

#3 - c4-judge

2024-05-23T13:47:20Z

alcueca changed the severity to 3 (High Risk)

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L287-L326

Vulnerability details

  • Upon calculating the TVL of the system, the ETH balance held by WithdrawQueue should also be accounted, since the shares are not burnt, when the request is submitted. However, the total value in eth held by WithdrawQueue is computed odLength * tokenLength times, when it is supposed to be simply just tokenLength.

Impact

  • The users will face unfair exchange rates during depositing/withdrawing funds.
  • Short term:

          // withdrawalQueue total value
          uint256 totalWithdrawalQueueValue = 0;
    
    +        for (uint i = 0; i < collateralTokens.length; i++) {
    +            totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
    +                collateralTokens[i], 
    +                collateralTokens[i].balanceOf(withdrawQueue)
    +            );
    +        }
    
    
          for (uint256 i = 0; i < odLength; ) {
              // Track the TVL for this OD
              uint256 operatorTVL = 0;
    
              // Track the individual token TVLs for this OD - native ETH will be last item in the array
              uint256[] memory operatorValues = new uint256[](collateralTokens.length + 1);
              operatorDelegatorTokenTVLs[i] = operatorValues;
    
              // Iterate through the tokens and get the value of each
              uint256 tokenLength = collateralTokens.length;
              for (uint256 j = 0; j < tokenLength; ) {
                  // Get the value of this token
    
                  uint256 operatorBalance = operatorDelegators[i].getTokenBalanceFromStrategy(
                      collateralTokens[j]
                  );
    
                  // Set the value in the array for this OD
                  operatorValues[j] = renzoOracle.lookupTokenValue(
                      collateralTokens[j],
                      operatorBalance
                  );
    
                  // Add it to the total TVL for this OD
                  operatorTVL += operatorValues[j];
    
    -                // record token value of withdraw queue
    -                if (!withdrawQueueTokenBalanceRecorded) { // fixme: high - accounted collToken.length * OD.length
    -                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
    -                        collateralTokens[i], 
    -                        collateralTokens[j].balanceOf(withdrawQueue)
    -                    );
    -                }
    
                  unchecked {
                      ++j;
                  }
              }
  • Long term:

    • Consider caching all LST/ETH exchange rates first, computing the value held by WtihdrawQueue, and then looping over the ods.

Assessed type

Math

#0 - c4-judge

2024-05-16T10:32:33Z

alcueca marked the issue as satisfactory

#1 - c4-judge

2024-05-16T10:32:39Z

alcueca marked the issue as partial-50

#2 - alcueca

2024-05-16T10:33:27Z

Not sure the warden pointed at the right problem, or the right solution, but it's in the general neighbourhood.

#3 - c4-judge

2024-05-16T10:36:47Z

alcueca marked the issue as satisfactory

#4 - c4-judge

2024-05-16T10:38:47Z

alcueca changed the severity to 2 (Med Risk)

#5 - c4-judge

2024-05-16T10:39:08Z

alcueca changed the severity to 3 (High Risk)

#6 - c4-judge

2024-05-20T04:26:26Z

alcueca changed the severity to 2 (Med Risk)

#7 - c4-judge

2024-05-23T13:47:20Z

alcueca changed the severity to 3 (High Risk)

Awards

0.0402 USDC - $0.04

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_20_group
duplicate-198

External Links

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L558-L562 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L143-L154

Vulnerability details

  • When user deposits whitelisted underlying assets, the system chooses an operatorDelegator who is able to accept the deposit. But prior to the transfer, the RestakeManager ensures that there is currently no deficit in WithdrawalQueue. In order to do that, bufferToFill is being subtracted from an initial user deposit. However, if the bufferToFill equals to an amount being supplied by user, a zero amount of underlying assets will be deposited to the selected opertatorDelegator, which reverts in such a case.

Impact

  • DoS
  • Short term:
    
              uint256 bufferToFill = depositQueue.withdrawQueue().getBufferDeficit(
              address(_collateralToken)
          );
          if (bufferToFill > 0) {
              bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;
              // update amount to send to the operator Delegator
              _amount -= bufferToFill;
    
              // safe Approve for depositQueue
              _collateralToken.safeApprove(address(depositQueue), bufferToFill);
    
              // fill Withdraw Buffer via depositQueue
              depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
          }
    
    +        if (_amount > 0) {
              // Approve the tokens to the operator delegator
              _collateralToken.safeApprove(address(operatorDelegator), _amount);
    
              // Call deposit on the operator delegator
              operatorDelegator.deposit(_collateralToken, _amount);
    +        }
    
    
          // Calculate how much ezETH to mint
          uint256 ezETHToMint = renzoOracle.calculateMintAmount(
              totalTVL,
              collateralTokenValue,
              ezETH.totalSupply()
          );

Assessed type

Context

#0 - c4-judge

2024-05-20T05:03:08Z

alcueca marked the issue as satisfactory

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter