A MALICIOUS WHALE CAN `DoS` THE ASSET DEPOSIT FUNCTIONALITY OF THE `LRTDepositPool` CONTRACT BY DIRECLTY TRANSFERRING A LARGE AMOUNT OF RESPECTIVE TOKENS TO THE PROTOCOL

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L132-L134 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L56-L58 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L47-L51 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L79 https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L83

Vulnerability details

Impact

The LRTDepositPool.depositAsset function is used to allow user to deposit particular asset amount to the LRTDepositPool contract. In the depositAsset function there is a deposit limit check as shown below:

    if (depositAmount > getAssetCurrentLimit(asset)) { //@audit-info - deposit amount should not exceed current avaialble limit
        revert MaximumDepositLimitReached();
    }

The above check ensures that the depositAmount of the msg.sender should not exceed the currently available deposit limit of the asset. The function logic of the LRTDepositPool.getAssetCurrentLimit is shown below:

function getAssetCurrentLimit(address asset) public view override returns (uint256) {
    return lrtConfig.depositLimitByAsset(asset) - getTotalAssetDeposits(asset); //@audit-info - get current available limit of asset deposit
}

The above getTotalAssetDeposits function call gets the currently deposited asset amounts in deposit pool, all node delegator contracts and eigenlayer as shown below;

function getTotalAssetDeposits(address asset) public view override returns (uint256 totalAssetDeposit) { //@audit-info - return teh total assets present in the protocol
    (uint256 assetLyingInDepositPool, uint256 assetLyingInNDCs, uint256 assetStakedInEigenLayer) =
        getAssetDistributionData(asset);
    return (assetLyingInDepositPool + assetLyingInNDCs + assetStakedInEigenLayer); //@audit-info - add the asset amounts lying in each of the above three places and return it
}

The deposited asset amounts in each of the above contracts are calculated in the LRTDepositPool.getAssetDistributionData function as shown below:

    assetLyingInDepositPool = IERC20(asset).balanceOf(address(this)); //@audit-info - get the balance of the asset in this contract

    uint256 ndcsCount = nodeDelegatorQueue.length; //@audit-info - nodeDelegatorQueue is an address array and its length is taken here
    for (uint256 i; i < ndcsCount;) { //@audit-info - loop through the node delegators
        assetLyingInNDCs += IERC20(asset).balanceOf(nodeDelegatorQueue[i]); //@audit-info - add up the asset balances in each of the node delegators
        assetStakedInEigenLayer += INodeDelegator(nodeDelegatorQueue[i]).getAssetBalance(asset); //@audit-info - add up the assets staked in the eigen layer by the node delegator addresses
        unchecked {
            ++i;
        } //@audit-info - increment the i
    }

The issue here is that both the assetLyingInDepositPool and the assetLyingInNDCs values are calculated by using the IERC20(asset).balanceOf() function thus getting the available asset balance of the contract at the time of the function call. As a result a malicious whale can easily deposit a large amount of the corresponding asset to either LRTDepositPool contract or to any of the Node Delegator Contracts and increase the value returned from the IERC20(asset).balanceOf() calls. This will increase the getTotalAssetDeposits(asset) value and revert the getAssetCurrentLimit function call if the lrtConfig.depositLimitByAsset(asset) < getTotalAssetDeposits(asset). As a result the subsequent asset deposits to the protocol via the LRTDepositPool.depositAsset function will be DoS for the other users since the call to the LRTDepositPool.getAssetCurrentLimit function will revert due to underflow.

Hence a whale can DoS the deposit functionality of the LRTDepositPool contract (LRTDepositPool.depositAsset function) by depositing a large amount of asset token to either the LRTDepositPool contract or to any of the Node Delegator Contracts directly.

Proof of Concept

        if (depositAmount > getAssetCurrentLimit(asset)) {
            revert MaximumDepositLimitReached();
        }

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L132-L134

    function getAssetCurrentLimit(address asset) public view override returns (uint256) {
        return lrtConfig.depositLimitByAsset(asset) - getTotalAssetDeposits(asset);
    }

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L56-L58

    function getTotalAssetDeposits(address asset) public view override returns (uint256 totalAssetDeposit) {
        (uint256 assetLyingInDepositPool, uint256 assetLyingInNDCs, uint256 assetStakedInEigenLayer) =
            getAssetDistributionData(asset);
        return (assetLyingInDepositPool + assetLyingInNDCs + assetStakedInEigenLayer);
    }

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L47-L51

        assetLyingInDepositPool = IERC20(asset).balanceOf(address(this));

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L79

            assetLyingInNDCs += IERC20(asset).balanceOf(nodeDelegatorQueue[i]);

https://github.com/code-423n4/2023-11-kelp/blob/main/src/LRTDepositPool.sol#L83

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to keep a track of deposits made to the LRTDepositPool contract and Node Delegator Contracts in a mapping (for each asset and for each user) while the assets are being deposited in the respective functions and then use the deposited amounts of in the mapping to calculate the getTotalAssetDeposits value in the LRTDepositPool.getAssetCurrentLimit with out using the IERC20(asset).balanceOf() function calls in the LRTDepositPool.getAssetDistributionData function. This way a malicious whale user will not be able to manipulate the getTotalAssetDeposits value since it is calculated from the values in the mapping which were updated when assets were deposited via the valid functions such as LRTDepositPool.depositAsset and LRTDepositPool.transferAssetToNodeDelegator.

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #435

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid