Kelp DAO | rsETH - taner2344's results

Lines of code

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

Vulnerability details

Impact

On LRTConfig contract The method updateAssetDepositLimit is compromising protocol availability on LRTConfig contract by locking the depositAsset method in an unwanted way.

Proof of Concept

On LRTConfig contract , The method updateAssetDepositLimit is coordinating and preventing exceeding deposits to the protocol through depositLimitByAsset state variable.

On LRTConfig contract L94-L104 :

    function updateAssetDepositLimit(   
	address asset,
        uint256 depositLimit
    ) external onlyRole(LRTConstants.MANAGER) onlySupportedAsset(asset) {
        depositLimitByAsset[asset] = depositLimit;
        emit AssetDepositLimitUpdate(asset, depositLimit);
    }

On the other hand , on LRTDepositPool contract limit of asset that will deposited is calculating as follow:

on LRTDepositPool contract L56-L58 :

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

As you can see , while calculating getAssetCurrentLimit getTotalAssetDeposits subtracts from depositLimitByAsset(asset) variable. Note that getTotalAssetDeposits value. should not be higher than lrtConfig.depositLimitByAsset(asset) .

Nevertheless, on the detail of getTotalAssetDeposits method :

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

It is clear that getTotalAssetDeposits mostly representing amount of ETH that is controlled by other protocols. That means reducing lrtConfig.depositLimitByAsset(asset) would cause to negative value on getAssetCurrentLimit and it will revert immediately. To prevent this updateAssetDepositLimit method should not allow reducing deposit limit.

Tools Used

Manual review

Recommended Mitigation Steps

On LRTConfig contract add following line between L101-L102

function updateAssetDepositLimit( 
        address asset,
        uint256 depositLimit
    ) external onlyRole(LRTConstants.MANAGER) onlySupportedAsset(asset) {

+++ require(depositLimit > depositLimitByAsset[asset] , “new limit must be higher than old one “);

        depositLimitByAsset[asset] = depositLimit;
        emit AssetDepositLimitUpdate(asset, depositLimit);
    }




## Assessed type

Other