zzykxx - `RsETHAdapter` adapter `_stake()` function lacks slippage control

[sherlock-admin3]

zzykxx

medium

RsETHAdapter adapter _stake() function lacks slippage control

Summary

Vulnerability Detail

The function RsETHAdapter::_stake() deposits assets into Kelp by calling the depositETH function:

    function _stake(uint256 stakeAmount) internal override returns (uint256) {
        ...SNIP...
        // 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;
    }

The first parameter is supposed to be used for slippage control, but Napier hardcoded the value to 0 resulting no restrictions over the amount of rsETH minted by the function. Because of this the user trying to stake ETH in the Napier protocol could end up receiving less/more PT/YT tokens than expected.

Impact

Users staking via the RsETHAdapter have no control over the slippage they will be subject to, which can lead to them receiving less/more PT/YT tokens than expeced.

Code Snippet

Tool used

Manual Review

Recommendation

Implement slippage control in the RsETHAdapter adapter.

Duplicate of #26

[massun-onibakuchi]

On our side, users interact with TrancheRouter.sol in v1-pool repo implementing such slippage protection. https://github.com/sherlock-audit/2024-05-napier-update/blob/main/metapool-router/lib/v1-pool/src/TrancheRouter.sol

[merlin-up]

Escalate

Duplicate of #26

[sherlock-admin3]

Escalate

Duplicate of #26

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]

I agree that this is a duplicate of #26. But if we look at getRsETHAmountToMint implementation. minted rsEth = msg.value * ethPrice / rsEthPrice . And because integrate protocols and their oracles are TRUSTED. so depegs are not concern.

In my opinion #26 and it's duplicates are valid Low/Information and should get closed.

[WangSecurity]

I think this issue is indeed borderline low/medium. As correctly noted by the Lead Judge, oracles are trusted to provide correct price (regarding depegs). And if we consider the "attack" to make the user lose value, then as I understand rsETH contract has it's protections.

Still it doesn't change the fact that this is susceptible to sharp price increases or decreases. Hence, the price of the asset can increase/decrease in the same block, right before the user's stake call goes through, leading to users getting less/more tokens (increase/decrease in price respectively) than they initially wanted to. The exact scenario with explanation of impact is well explained in #26. Hence, I believe the issue is sufficient to be of medium severity.

Agree with the escalation that this report should be duplicated with #26, planning to accept.

[merlin-up]

@WangSecurity The last thing I want to mention about the duplicates of this report (#20 and #56): will these duplicates also become duplicates of report #26?

[WangSecurity]

@Soloviy yes they will also be duplicated with 26, since 88 will be the duplicate.

[Evert0x]

Result: Medium Duplicate of #26

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• merlin-up: accepted