An asset's depositLimit can be set as < TotalAssetDeposits

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

The method updateAssetDepositLimit() doesn't check the TotalAssetDeposits. It can be set as < TotalAssetDeposits.

Vulnerability Details

The method updateAssetDepositLimit() sets the input amount as depositLimit for an asset. It doesn't check the total assets Deposited. If set incorrectly can cause depositAsset() and getAssetCurrentLimit() to revert.

POC Test

Add the following test in LRTDepositPoolTest.t.sol

function test_GetAssetCurrentLimitUnderflows() external {
        vm.startPrank(manager);
        lrtConfig.updateAssetDepositLimit(address(stETH), 10 ether);
        vm.stopPrank();

        // deposit 1 ether stETH
        vm.startPrank(alice);
        stETH.approve(address(lrtDepositPool), 6 ether);
        lrtDepositPool.depositAsset(address(stETH), 6 ether);
        vm.stopPrank();

        vm.startPrank(manager);
        lrtConfig.updateAssetDepositLimit(address(stETH), 5 ether);
        vm.stopPrank();

        // vm.expectRevert();
        lrtDepositPool.getAssetCurrentLimit(address(stETH));
    }

POC Logs

Encountered 1 failing test in test/LRTDepositPoolTest.t.sol:LRTDepositPoolGetAssetCurrentLimit
[FAIL. Reason: Arithmetic over/underflow] test_GetAssetCurrentLimitUnderflows() (gas: 217600)

Impact

If the depositLimit is set as < TotalAssetDeposits then getAssetCurrentLimit() will underflow. Which will cause depositAsset() to revert.

Recommendations

Ensure that depositLimit >= TotalAssetDeposits while setting in updateAssetDepositLimit()

Assessed type

Under/Overflow

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #319

[raymondfam]

TotalAssetDeposits does not come into play till users start depositing LST where LSTs deposited by users will end up on that LST asset strategy contract in the Eigenlayer protocol.

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid