zzykxx - Kelp adapter won't allow users to deposit in some situations

[sherlock-admin3]

zzykxx

medium

Kelp adapter won't allow users to deposit in some situations

Summary

Users will be unable to deposit into the Kelp adapter in some situations due to the _stake() function reverting.

Vulnerability Detail

The function RsETHAdapter::_stake() reverts if the passed stakeAmount is lower than the current minAmountToDeposit of Kelp:

function _stake(uint256 stakeAmount) internal override returns (uint256) {
    if (stakeAmount == 0) return 0;

    // Check LRTDepositPool stake limit
    uint256 stakeLimit = RSETH_DEPOSIT_POOL.getAssetCurrentLimit(Constants.ETH);
    if (stakeAmount > stakeLimit) {
        // Cap stake amount
        stakeAmount = stakeLimit;
    }
    // Check LRTDepositPool minAmountToDeposit
@>  if (stakeAmount <= RSETH_DEPOSIT_POOL.minAmountToDeposit()) revert MinAmountToDepositError();
    // Check paused of LRTDepositPool
    if (RSETH_DEPOSIT_POOL.paused()) revert ProtocolPaused();

    // Interact
    IWETH9(Constants.WETH).withdraw(stakeAmount);
    uint256 _rsETHAmt = RSETH.balanceOf(address(this));
    RSETH_DEPOSIT_POOL.depositETH{value: stakeAmount}(0, REFERRAL_ID);
    _rsETHAmt = RSETH.balanceOf(address(this)) - _rsETHAmt;

    if (_rsETHAmt == 0) revert InvariantViolation();

    return stakeAmount;
}

An user that is trying to deposit has not full control of the value of the stakeAmount, because the variable is dependant on the current bufferEthCache, queueEthCache and targetBufferEth as can be seen in the RsETHAdapter::prefundedDeposit() function:

...SNIP...
uint256 targetBufferEth = ((totalAssets() + assets) * $.targetBufferPercentage) / BUFFER_PERCENTAGE_PRECISION;

uint256 availableEth = bufferEthCache + assets; // non-zero

StakeLimitTypes.Data memory data = $.packedStakeLimitData.getStorageStakeLimitStruct();
if (targetBufferEth >= availableEth + queueEthCache || data.isStakingPaused()) {
    $.bufferEth = availableEth.toUint128();
    return (assets, shares);
}

uint256 stakeAmount;
unchecked {
    stakeAmount = availableEth + queueEthCache - targetBufferEth;
}

if (stakeAmount > availableEth) {
    stakeAmount = availableEth;
}

...SNIP...

stakeAmount = _stake(stakeAmount); // stake amount can be 0

This results in the RsETHAdapter::_stake() function potentially reverting if the calculated stakeAmount is lower than Kelp current minAmountToDeposit.

Impact

Users won't not be able to deposit funds in the RsETHAdapter if this situation manifests.

Code Snippet

Tool used

Manual Review

Recommendation

In RsETHAdapter::_stake() return 0 if the calculated stakeAmount is lower than minAmountToDeposit:

function _stake(uint256 stakeAmount) internal override returns (uint256) {
    if (stakeAmount == 0) return 0;

    // Check LRTDepositPool stake limit
    uint256 stakeLimit = RSETH_DEPOSIT_POOL.getAssetCurrentLimit(Constants.ETH);
    if (stakeAmount > stakeLimit) {
        // Cap stake amount
        stakeAmount = stakeLimit;
    }
    // Check LRTDepositPool minAmountToDeposit
@>  if (stakeAmount <= RSETH_DEPOSIT_POOL.minAmountToDeposit()) return 0;
    // Check paused of LRTDepositPool
    if (RSETH_DEPOSIT_POOL.paused()) revert ProtocolPaused();

    // Interact
    IWETH9(Constants.WETH).withdraw(stakeAmount);
    uint256 _rsETHAmt = RSETH.balanceOf(address(this));
    RSETH_DEPOSIT_POOL.depositETH{value: stakeAmount}(0, REFERRAL_ID);
    _rsETHAmt = RSETH.balanceOf(address(this)) - _rsETHAmt;

    if (_rsETHAmt == 0) revert InvariantViolation();

    return stakeAmount;
}

RsETHAdapter.sol#L77

Duplicate of #46

[sherlock-admin3]

1 comment(s) were left on this issue during the judging contest.

z3s commented:

46

[WangSecurity]

46 will be the main issue for a new family, hence, this, as a duplicate of 46, will be valid as well.