merlin - Issue with staking being paused and LRTDepositPool limit in RsETHAdapter.sol

[sherlock-admin2]

merlin

medium

Issue with staking being paused and LRTDepositPool limit in RsETHAdapter.sol

Summary

RsETHAdapter does not stake any of the deposit but increases the buffer variable when isStakingPaused is true. This can lead to a situation where the WETH tokens in the buffer exceed the current staking limit in LRTDepositPool, resulting in eRsETH becoming less valuable and earning fewer rewards.

Vulnerability Detail

Currently, the maximum amount of ETH that all users can deposit in Kelp DAO is 100,000 ETH. Let's consider a scenario: Assume that the current staking limit of LRTDepositPool is 1 ETH.

1)The owner calls pauseStaking in RsETHAdapter.sol. 2)When users stake WETH while staking is paused, the bufferEth value increases. For example, during the period when staking was paused, the bufferEth increased by 10 ETH.

if (targetBufferEth >= availableEth + queueEthCache || data.isStakingPaused()) {
            /// WRITE ///
            $.bufferEth = availableEth.toUint128();
            return (assets, shares);
        }

3)From this, it can be understood that a significant portion of WETH cannot be deposited in LRTDepositPool, which will result in all depositors starting to earn fewer rewards.

Impact

Users will earn rewards that are lower than expected.

Code Snippet

src/adapters/BaseLSTAdapterUpgradeable.sol#L121-L125

Tool used

Manual Review

Recommendation

It's difficult to say how to best fix this, as this limit would need to be checked in the prefundedDeposit function. However, since the contract is paused, RSETH_DEPOSIT_POOL limit can change at any time which would not solve the issue.

[z3s]

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

[merlin-up]

Escalate I agree with @z3s that when staking is paused, the buffer will increase as users stake, and that is a design choice. However, I disagree with the assertion that there will be no loss.

From this, it can be understood that a significant portion of WETH cannot be deposited in LRTDepositPool

uint256 stakeLimit = RSETH_DEPOSIT_POOL.getAssetCurrentLimit(Constants.ETH);
        if (stakeAmount > stakeLimit) {
            // Cap stake amount
            stakeAmount = stakeLimit;
        }

Under ideal conditions, the ratio of [WETH in RsETHAdapter : WETH in RSETH_DEPOSIT_POOL] is [10% : 90%]. The report indicates that stakers will start receiving fewer rewards because the eRsETH share will depreciate as the ratio of [WETH in RsETHAdapter to WETH in RSETH_DEPOSIT_POOL] changes. This will make it less advantageous for stakers to continue holding PT + YT of RsETHAdapter.

Regarding the recommended mitigation step, I'm not sure what the best approach for the protocol is to avoid this situation, but I am open to discussing it further. I hope I was able to clarify my point.

[sherlock-admin3]

Escalate I agree with @z3s that when staking is paused, the buffer will increase as users stake, and that is a design choice. However, I disagree with the assertion that there will be no loss.

From this, it can be understood that a significant portion of WETH cannot be deposited in LRTDepositPool

uint256 stakeLimit = RSETH_DEPOSIT_POOL.getAssetCurrentLimit(Constants.ETH);
        if (stakeAmount > stakeLimit) {
            // Cap stake amount
            stakeAmount = stakeLimit;
        }

Under ideal conditions, the ratio of [WETH in RsETHAdapter : WETH in RSETH_DEPOSIT_POOL] is [10% : 90%]. The report indicates that stakers will start receiving fewer rewards because the eRsETH share will depreciate as the ratio of [WETH in RsETHAdapter to WETH in RSETH_DEPOSIT_POOL] changes. This will make it less advantageous for stakers to continue holding PT + YT of RsETHAdapter.

Regarding the recommended mitigation step, I'm not sure what the best approach for the protocol is to avoid this situation, but I am open to discussing it further. I hope I was able to clarify my point.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[z3s]

Design decisions are not valid issues. Even if the design is suboptimal, but doesn't imply any loss of funds, these issues are considered informational.

This will make it less advantageous for stakers to continue holding PT + YT of RsETHAdapter.

This not loss of funds, it's Even if the design is suboptimal. I disagree with escalation, this issue is Low/Informational.

[WangSecurity]

Firstly, it's the TRUSTED admin who sets the pause, so this rule should apply:

An admin action can break certain assumptions about the functioning of the code. Example: Pausing a collateral causes some users to be unfairly liquidated or any other action causing loss of funds. This is not considered a valid issue

Secondly, I agree that this is a design decision and that there are no funds lost. Yes, the stakers earn less rewards, but it's an opportunity loss and as I understand they can withdraw anytime.

Planning to reject the escalation and leave the issue as it is.

[Evert0x]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• Soloviy: rejected