Kelp DAO | rsETH - 0xLeveler's results

Lines of code

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

Vulnerability details

Impact

Prevention of User Deposit and Denial-of-service in depositAsset due to insufficient input validation in updateAssetDepositLimit

Proof of Concept

say User(Alice) wants to deposit her LST into the Kelp protocol, she calls depositAsset to no avail, because depositAsset will revert and prevent her deposit if

• LRTManager(compromised or not) sets the depositLimit of an asset to a value less than getTotalAssetDeposits(asset) in getAssetCurrentLimit function. Due to insufficient input validation which allows this, all deposits will revert.

1. User tries to deposit

function depositAsset(
        address asset,
        uint256 depositAmount
    )
        external
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
    {
        // checks
        if (depositAmount == 0) {
            revert InvalidAmount();
        }
        if (depositAmount > getAssetCurrentLimit(asset)) { //@audit check reverts here as condition fails
            revert MaximumDepositLimitReached();
        }

2. The second if condition checks that the depositAmount is greater than getAssetCurrentLimit(asset), it reads the getAssetCurrentLimit(asset) from :

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

The condition for this function to not revert hinges on lrtConfig.depositLimitByAsset >= getTotalAssetDeposits(asset) but this can occur if depositLimit is erroneously updated to a lower value in the function below, because there are no validation checks.

function updateAssetDepositLimit(
        address asset,
        uint256 depositLimit
    )
        external
        onlyRole(LRTConstants.MANAGER)
        onlySupportedAsset(asset)
    {
        //@audit some input validation should be applied here to prevert DOS of depositAsset function in 
        LRTDepositPool.sol or elsewhere
        depositLimitByAsset[asset] = depositLimit; 
        emit AssetDepositLimitUpdate(asset, depositLimit);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Add some form of validation to the UpdateAssetDepositLimit function in LRTConfig: depositLimit >= LRTDepositPool.getTotalAssetDeposit()

Assessed type

Invalid Validation