Skip to content
This repository has been archived by the owner on Nov 24, 2024. It is now read-only.

PNS - Incorrect staking limit check in RsETHAdapter #82

Closed
sherlock-admin2 opened this issue May 20, 2024 · 1 comment
Closed

PNS - Incorrect staking limit check in RsETHAdapter #82

sherlock-admin2 opened this issue May 20, 2024 · 1 comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin2
Copy link

sherlock-admin2 commented May 20, 2024

PNS

medium

Incorrect staking limit check in RsETHAdapter

Summary

The adapter checks the staking limit using a closed condition <=, which results in a deposit equal to the minimum deposit being rejected. However, this is not accurate behavior for the integrated protocol.

Vulnerability Detail

File: napier-uups-adapters/src/adapters/kelp/RsETHAdapter.sol
67:    function _stake(uint256 stakeAmount) internal override returns (uint256) {
...
77:         if (stakeAmount <= RSETH_DEPOSIT_POOL.minAmountToDeposit()) revert MinAmountToDepositError(); //@audit limit check
...
90:    }

https://github.com/sherlock-audit/2024-05-napier-update/blob/c31af59c6399182fd04b40530d79d98632d2bfa7/napier-uups-adapters/src/adapters/kelp/RsETHAdapter.sol#L77

The issue arises because the integrated protocol considers the minimum deposit as acceptable.

if (depositAmount == 0 || depositAmount < minAmountToDeposit) {
            revert InvalidAmountToDeposit();
        }

https://github.com/Kelp-DAO/LRT-rsETH/blob/e75e9ef168a7b192abf76869977cd2ac8134849c/contracts/LRTDepositPool.sol#L200C51-L200C69

Impact

The minimum deposit amount for a user attempting to deposit into the KelpDAO protocol will always be rejected.

Code Snippet

Tool used

Manual Review

Recommendation

The condition should be changed to an open <.

Additional info:

Medium severity because in the function documentation _stake, it is described that limits should be checked to prevent DoS. Since only the update is audited and not the entire protocol, it is difficult to assess the specific DoS, but it should be assumed that checking the limits should be accurate and consistent with the integrated protocol.

File: napier-uups-adapters/src/adapters/kelp/RsETHAdapter.sol
64:     /// @notice Kelp allows ETH, ETHx, stETH or sfrxETH via LRTDepositPool.
65:     /// @dev Kelp has a limit on the amount of ETH that can be staked.
66:     /// @dev Need to check the current staking limit before staking to prevent DoS.
67:     function _stake(uint256 stakeAmount) internal override returns (uint256) {

Duplicate of #46

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 22, 2024
@z3s z3s removed the Medium A valid Medium severity issue label May 24, 2024
@sherlock-admin2 sherlock-admin2 changed the title Rhythmic Shamrock Eel - Incorrect staking limit check in RsETHAdapter PNS - Incorrect staking limit check in RsETHAdapter May 24, 2024
@sherlock-admin2 sherlock-admin2 added Non-Reward This issue will not receive a payout and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 24, 2024
@WangSecurity
Copy link

#54 will be duplicated with #46, hence, this report will also be duplicated with #46.

@Evert0x Evert0x added the Medium A valid Medium severity issue label Jun 1, 2024
@sherlock-admin2 sherlock-admin2 added Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels Jun 1, 2024
@sherlock-admin4 sherlock-admin4 added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

5 participants