Renzo - zhaojohnson's results

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L299-L321

Vulnerability details

Impact

Use incorrect loop index to calculate the tvl. This will cause inaccurate tvl value.

Proof of Concept

In RestakeManager::calculateTVLs(), the function will calculate tvls from all operator delegators and deposit queues, withdraw queues. When the function calculates withdraw queues' token values, it uses collateralTokens[i] and collateralTokens[j].balanceOf(withdrawQueue), this is incorrect. We should use the same collateral and same collateral's amount to calculate tvl. In one word, we should change collateralTokens[i] to collateralTokens[j].

    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
            ...
            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) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
              @==>      collateralTokens[i],
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );
                }
                unchecked {
                    ++j;
                }
            }

Tools Used

Manual

Recommended Mitigation Steps

Change collateralTokens[i] to collateralTokens[j].

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

Chainlink price's update will cause the whole tvl's change. The whole tvl value's change will cause share price's change.

Proof of Concept

Depositors will earn profit when the share price increases. When chainlink updates the related feeds, the whole tvl will change because collateral token's price changes. Let's take stETH/ETH as one example. stETH/ETH feed's deviation threshold is 0.5%. It means tvl's value can increase 0.5% in one block in some extreme condition. This will be one possible arbitrage chance.

Possible arbitrage attack vector:

• Alice deposits collateral tokens into renzo by frontrun.
• Chainlink update stETH/ETH feed. For example, the feed's price increase 0.5% compared with last update. Now the whole tvl will increase because of stETH/ETH price increase. For example, the whole tvl increase 0.2%.
• Alice withdraw her shares with the increased price.

    function lookupTokenValue(IERC20 _token, uint256 _balance) public view returns (uint256) {
        AggregatorV3Interface oracle = tokenOracleLookup[_token];
        if (address(oracle) == address(0x0)) revert OracleNotFound();

        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
        if (price <= 0) revert InvalidOraclePrice();

        // Price is times 10**18 ensure value amount is scaled
        return (uint256(price) * _balance) / SCALE_FACTOR;
    }

Tools Used

Manual

Recommended Mitigation Steps

Consider to lock a period for the deposit to avoid the arbitrage.

Assessed type

Timing