Renzo - maxim371's results

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Delegation/OperatorDelegator.sol#L329-L337

Vulnerability details

Impact

queuedShares[address(this)] == 0: This line checks if there are any queued shares associated with the contract's address itself. This is an error because typically, the queued shares should be checked against the token's address, not the contract's address. The correct check should likely be queuedShares[address(token)] == 0. This leads to incorrect balance calculation.

Proof of Concept

        `queuedShares[address(this)] == 0
            ? tokenStrategyMapping[token].userUnderlyingView(address(this))
            : tokenStrategyMapping[token].userUnderlyingView(address(this)) +
                tokenStrategyMapping[token].sharesToUnderlyingView(
                    queuedShares[address(token)]
                );
}


/// @dev Gets the amount of ETH staked in the EigenLayer`

`// SPDX-License-Identifier: MIT pragma solidity ^0.8.13;

interface ITokenStrategy { function userUnderlyingView(address user) external view returns (uint256); function sharesToUnderlyingView(uint256 shares) external view returns (uint256); }

contract TokenManager { mapping(address => ITokenStrategy) public tokenStrategyMapping; mapping(address => uint256) public queuedShares;

function getTokenBalanceFromStrategy(address token) external view returns (uint256) {
    uint256 underlyingBalance = tokenStrategyMapping[token].userUnderlyingView(address(this));
    uint256 shareValue = tokenStrategyMapping[token].sharesToUnderlyingView(queuedShares[token]);
    return queuedShares[token] == 0 ? underlyingBalance : underlyingBalance + shareValue;
}

}`

`// SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.13;

import "forge-std/Test.sol"; import "../src/TokenManager.sol";

contract TokenManagerTest is Test { TokenManager public tokenManager; address tokenAddress = address(0x123); // Example token address address anotherTokenAddress = address(0x456); // Another token address for control

function setUp() public {
    tokenManager = new TokenManager();
    // Mock strategy setup
    ITokenStrategy strategy = new MockTokenStrategy();
    tokenManager.tokenStrategyMapping[tokenAddress] = strategy;
    tokenManager.tokenStrategyMapping[anotherTokenAddress] = strategy;
    // Set queued shares for the correct token address
    tokenManager.queuedShares[tokenAddress] = 1000;
}

function testCorrectAddressUsageForQueuedShares() public {
    // This should use the correct token address and not address(this)
    uint256 balance = tokenManager.getTokenBalanceFromStrategy(tokenAddress);
    assertEq(balance, expectedValue, "Balance does not match expected value with correct address handling.");
}

}

contract MockTokenStrategy is ITokenStrategy { function userUnderlyingView(address user) external pure returns (uint256) { return 500; // Example fixed return value }

function sharesToUnderlyingView(uint256 shares) external pure returns (uint256) {
    return shares * 2; // Example conversion logic
}

}`

Tools Used

Manual Review

Recommended Mitigation Steps

+ queuedShares[address(token)] == 0

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(collateralTokens[i], collateralTokens[j].balanceOf(withdrawQueue)); This calculates the total value of each token in the withdraw queue, but it contains a bug where collateralTokens[i] should be collateralTokens[j].

Proof of Concept

`function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
    uint256[][] memory operatorDelegatorTokenTVLs = new uint256[][](operatorDelegators.length);
    uint256[] memory operatorDelegatorTVLs = new uint256[](operatorDelegators.length);
    uint256 totalTVL = 0;


    // Iterate through the ODs
    uint256 odLength = operatorDelegators.length;


    // flag for withdrawal queue balance set
    bool withdrawQueueTokenBalanceRecorded = false;
    address withdrawQueue = address(depositQueue.withdrawQueue());


    // withdrawalQueue total value
    uint256 totalWithdrawalQueueValue = 0;


    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) {
                totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                    collateralTokens[i],
                    collateralTokens[j].balanceOf(withdrawQueue)
                );
            }


            unchecked {
                ++j;
            }
        }


        // Get the value of native ETH staked for the OD
        uint256 operatorEthBalance = operatorDelegators[i].getStakedETHBalance();


        // Save it to the array for the OD
        operatorValues[operatorValues.length - 1] = operatorEthBalance;


        // Add it to the total TVL for this OD
        operatorTVL += operatorEthBalance;


        // Add it to the total TVL for the protocol
        totalTVL += operatorTVL;


        // Save the TVL for this OD
        operatorDelegatorTVLs[i] = operatorTVL;


        // Set withdrawQueueTokenBalanceRecorded flag to true
        withdrawQueueTokenBalanceRecorded = true;


        unchecked {
            ++i;
        }
    }


    // 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);


    return (operatorDelegatorTokenTVLs, operatorDelegatorTVLs, totalTVL);
}`

Suppose we have the following setup: Operator Delegators: 2 delegators Collateral Tokens: 3 tokens (Token A, Token B, Token C) When the outer loop (i) is at index 0 (first operator delegator) and the inner loop (j) iterates over the tokens, it should calculate the value of each token (A, B, C) in the withdraw queue. However, due to the incorrect indexing: For Token A (j = 0), it incorrectly uses collateralTokens[0] (which is correct in this specific case but coincidental). For Token B (j = 1), it incorrectly uses collateralTokens[0] instead of collateralTokens[1]. For Token C (j = 2), it incorrectly uses collateralTokens[0] instead of collateralTokens[2]. This results in the oracle always looking up the value of Token A for the balances of Tokens A, B, and C in the withdraw queue, leading to incorrect total value calculations.

`// SPDX-License-Identifier: MIT pragma solidity ^0.8.0;

import "forge-std/Test.sol"; import "../contracts/RestakeManager.sol"; import "../contracts/mocks/MockToken.sol"; import "../contracts/mocks/MockOracle.sol"; import "../contracts/mocks/MockOperatorDelegator.sol";

contract RestakeManagerTest is Test { RestakeManager restakeManager; MockToken[] tokens; MockOracle oracle; MockOperatorDelegator[] operatorDelegators; address withdrawQueue;

function setUp() public {
    // Set up the RestakeManager and other mocks
    restakeManager = new RestakeManager();
    oracle = new MockOracle();
    withdrawQueue = address(new MockToken()); // This will act as the withdraw queue

    // Create mock tokens and operator delegators
    for (uint i = 0; i < 3; i++) {
        MockToken token = new MockToken();
        tokens.push(token);
        MockOperatorDelegator delegator = new MockOperatorDelegator();
        operatorDelegators.push(delegator);
        restakeManager.addCollateralToken(token);
        restakeManager.addOperatorDelegator(delegator);
    }

    // Initialize balances and values in the oracle and delegators
    for (uint i = 0; i < tokens.length; i++) {
        tokens[i].mint(withdrawQueue, 1000 * 10**18); // 1000 tokens of each type in the withdraw queue
        oracle.setTokenValue(tokens[i], 1 ether); // 1 token = 1 ether
    }
}

function testCalculateTVLsCorrectlyAccountsForWithdrawQueue() public {
    // Call calculateTVLs
    (,, uint256 totalTVL) = restakeManager.calculateTVLs();

    // Expected TVL should include the value of tokens in the withdraw queue
    uint256 expectedTVL = 3000 ether; // 1000 tokens * 3 types * 1 ether each
    assertEq(totalTVL, expectedTVL, "The calculated TVL does not match the expected TVL including the withdraw queue");
}

}`

Tools Used

Manual Review

Recommended Mitigation Steps

+ totalWithdrawalQueueValue += renzoOracle.lookupTokenValue( collateralTokens[j], collateralTokens[j].balanceOf(withdrawQueue) );

Assessed type

Context