no - Checking return share in _stake() causes Dos

[sherlock-admin2]

no

medium

Checking return share in _stake() causes Dos

Summary

Checking return share in _stake() causes Dos

Vulnerability Detail

Let us review EETHAdapter.sol for example.

  function _stake(uint256 stakeAmount) internal override returns (uint256) {
        if (stakeAmount == 0) return 0;

        IWETH9(Constants.WETH).withdraw(stakeAmount);
        uint256 _eETHAmt = LIQUIDITY_POOL.deposit{value: stakeAmount}();

@>        if (_eETHAmt == 0) revert InvariantViolation();
        return stakeAmount;
    }

The _stake will revert in the condition that the stakeAmount is very small(but not zero) causing return deposited share is zero.
This could always happens. Because stakeAmount is not the user's input, it is calculate by this protocal.

function prefundedDeposit() external nonReentrant onlyTranche returns (uint256, uint256) {
        uint256 bufferEthCache = bufferEth; // cache storage reads
        uint256 queueEthCache = totalQueueEth; // cache storage reads
        uint256 assets = IWETH9(WETH).balanceOf(address(this)) - bufferEthCache; // amount of WETH deposited at this time
        uint256 shares = previewDeposit(assets);

        if (assets == 0) return (0, 0);
        if (shares == 0) revert ZeroShares();

        // Calculate the target buffer amount considering the user's deposit.
        // bufferRatio is defined as the ratio of ETH balance to the total assets in the adapter in ETH.
        // Formula:
        // desiredBufferRatio = (totalQueueEth + bufferEth + assets - s) / (totalQueueEth + bufferEth + stakedEth + assets)
        // Where:
        // assets := Amount of ETH the user is depositing
        // s := Amount of ETH to stake at this time, s <= bufferEth + assets.
        //
        // Thus, the formula can be simplified to:
        // s = (totalQueueEth + bufferEth + assets) - (totalQueueEth + bufferEth + stakedEth + assets) * desiredBufferRatio
        //   = (totalQueueEth + bufferEth + assets) - targetBufferEth
        //
        // Flow:
        // If `s` <= 0, don't stake any ETH.
        // If `s` < bufferEth + assets, stake `s` amount of ETH.
        // If `s` >= bufferEth + assets, all available ETH can be staked in theory.
        // However, we cap the stake amount. This is to prevent the buffer from being completely drained.
        //
        // Let `a` be the available amount of ETH in the buffer after the deposit. `a` is calculated as:
        // a = (bufferEth + assets) - s
        uint256 targetBufferEth = ((totalAssets() + assets) * targetBufferPercentage) / BUFFER_PERCENTAGE_PRECISION;

        /// WRITE ///
        _mint(msg.sender, shares);

        uint256 availableEth = bufferEthCache + assets; // non-zero

        // If the buffer is insufficient or staking is paused, doesn't stake any of the deposit
        StakeLimitTypes.Data memory data = packedStakeLimitData.getStorageStakeLimitStruct();
        if (targetBufferEth >= availableEth + queueEthCache || data.isStakingPaused()) {
            /// WRITE ///
            bufferEth = availableEth.toUint128();
            return (assets, shares);
        }

        // Calculate the amount of ETH to stake
        uint256 stakeAmount; // can be 0
        unchecked {
    @>        stakeAmount = availableEth + queueEthCache - targetBufferEth; // non-zero, no underflow
        }
        // If the calculated stake amount exceeds the available ETH, simply assign the available ETH to the stake amount.
        // Possible scenarios:
        // - Target buffer percentage was changed to a lower value and there is a large withdrawal request pending.
        // - There is a pending withdrawal request and the available ETH are not left in the buffer.
        // - There is no pending withdrawal request and the available ETH are not left in the buffer.
        if (stakeAmount > availableEth) {
            // Note: Admins should be aware of this situation and take action to refill the buffer.
            // - Pause staking to prevent further staking until the buffer is refilled
            // - Update stake limit to a lower value
            // - Increase the target buffer percentage
    @>        stakeAmount = availableEth; // All available ETH
        }

        // If the amount of ETH to stake exceeds the current stake limit, cap the stake amount.
        // This is to prevent the buffer from being completely drained. This is not a complete solution.
        uint256 currentStakeLimit = StakeLimitUtils.calculateCurrentStakeLimit(data); // can be 0 if the stake limit is exhausted
        if (stakeAmount > currentStakeLimit) {
    @>        stakeAmount = currentStakeLimit;
        }
        /// WRITE ///
        // Update the stake limit state in the storage
        packedStakeLimitData.setStorageStakeLimitStruct(data.updatePrevStakeLimit(currentStakeLimit - stakeAmount));

        /// 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 ///
        bufferEth = (availableEth - stakeAmount).toUint128(); // no underflow theoretically

        return (assets, shares);
    }

The stakeAmount could be zero and any very small value. _stake's codebase handles the case when stakeAmount is 0(if (stakeAmount == 0) return 0;), but does not handle the case when stakeAmount is very small.

The users deposit right value using Tranche, but could revert, and they don't konw why.
The same issue exists in UniETHAdapter.sol, RsETHAdapter.sol, PufETHAdapter.sol, RenzoAdapter.sol and RswETHAdapter.sol.

Impact

The users deposit right value using Tranche, but could revert, and they don't konw why.

Code Snippet

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/napier-v1/src/adapters/etherfi/EETHAdapter.sol#L123C1-L123C58

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/napier-v1/src/adapters/bedrock/UniETHAdapter.sol#L80C1-L80C54

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

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/napier-uups-adapters/src/adapters/puffer/PufETHAdapter.sol#L84C1-L84C58

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/napier-uups-adapters/src/adapters/renzo/RenzoAdapter.sol#L67C1-L67C74

https://github.com/sherlock-audit/2024-05-napier-update/blob/main/napier-uups-adapters/src/adapters/swell/RswETHAdapter.sol#L68C8-L68C58

Tool used

Manual Review

Recommendation

Pre Calculate the returned share , if it is zero, do not deposit. Just like stakeAmount is zero.

Duplicate of #64

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

z3s commented:

#46