search

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

8 stars 7 forks source link

blutorque - `currentStakeLimit` depletes faster in some adapters, due to actual amount spent less than the input `stakeAmount` #8

Open sherlock-admin2 opened 5 months ago

sherlock-admin2 commented 5 months ago

blutorque

medium

currentStakeLimit depletes faster in some adapters, due to actual amount spent less than the input stakeAmount

Summary

Vulnerability Detail

In the BaseLSTAdapterUpgradeable.prefundedDeposit(), the stake amount is capped to the currentStakeLimit.This is to prevent the buffer from being completed drained. 

      // Update the stake limit state in the storage
      $.packedStakeLimitData.setStorageStakeLimitStruct(data.updatePrevStakeLimit(currentStakeLimit - stakeAmount));

Before the staking occur, its checks whether the stakeAmount exceed current stakeLimit, if not modify the new stake limit to currentStakeLimit - stakeAmount. The issue is, the actual amount going to be spent could possibly lower than the stakeAmount

https://github.com/sherlock-audit/2024-05-napier-update/blob/c31af59c6399182fd04b40530d79d98632d2bfa7/napier-uups-adapters/src/adapters/BaseLSTAdapterUpgradeable.sol#L157-L158

        // Actual amount of ETH spent may be less than the requested amount.
        stakeAmount = _stake(stakeAmount); // stake amount can be 0

which means the stake limit that was updated previously does not account for the actual amount that we staked. I found one instance of adapters where this could possibly occur,

kelp/RsETHAdapter.sol: Input stakeAmount modified to lower value if its greater than the stakeLimit of RsETHDeposit pool,

Impact

With every prefundedDeposit call where excess WETH is left to stake, the stake limit will deplete faster.

Code Snippet

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

Tool used

Manual Review

Recommendation

The _stake() method do returns the actual spent amount, therefore I suggest to update the staking limit after the staking has been done. 

        /// INTERACT ///
        // Deposit into the yield source
        // Actual amount of ETH spent may be less than the requested amount.
        stakeAmount = _stake(stakeAmount); // stake amount can be 0

        /// WRITE ///
        // Update the stake limit state in the storage
        $.packedStakeLimitData.setStorageStakeLimitStruct(data.updatePrevStakeLimit(currentStakeLimit - stakeAmount));
sherlock-admin2 commented 5 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/napierfi/napier-v1/pull/219 https://github.com/napierfi/napier-uups-adapters/pull/9

sherlock-admin2 commented 4 months ago

The Lead Senior Watson signed off on the fix.