search

sherlock-audit / 2024-05-napier-update-judging

8 stars 7 forks source link

PNS - Incorrect staking limit check in `RsETHAdapter` #82

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 5 months ago

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

WangSecurity commented 4 months ago

https://github.com/sherlock-audit/2024-05-napier-update-judging/issues/54 will be duplicated with https://github.com/sherlock-audit/2024-05-napier-update-judging/issues/46, hence, this report will also be duplicated with https://github.com/sherlock-audit/2024-05-napier-update-judging/issues/46.