Adapters revert when 0 shares are minted, making it impossible to deposit under certain conditions

Summary

Users are unable to deposit into an Adapter in some situations due to the _stake() function reverting.

Vulnerability Detail

The function _stake() in all of the in-scope adapters reverts if the amounts of minted shares of the targeted protocol is 0.

Funds are deposited in an adapter via the prefundedDeposit() function, which internally calls _stake() by passing the amount to stake in the protocol, stakeAmount:

    ...SNIP...
    uint256 stakeAmount;
    unchecked {
        stakeAmount = availableEth + queueEthCache - targetBufferEth;
    }

    if (stakeAmount > availableEth) {
@>      stakeAmount = availableEth;
    }

    ...SNIP...
@>   stakeAmount = _stake(stakeAmount); // stake amount can be 0
    ...SNIP...

The amount to stake in the protocol, stakeAmount, can be restricted to availableEth. If availableEth/stakeAmount is low enough (but not 0) for the targeted protocol to mint 0 shares all of the adapters in-scope will revert by throwing an InvariantViolation(); error:

Impact

Users won't not be able to deposit funds if the stakeAmount is not enough to mint at least 1 share. The protocol genrally allows users to deposit both when stakeAmount is 0 and when the maximum deposit cap has been reached on the target protocol, which is incosistent with the behaviour outlined in this report.

A similar finding was disclosed in the previous Napier contest.

Code Snippet

Tool used

Manual Review

Recommendation

The function _stake() in the adapters should ensure that the shares minted are at least 1 before actually depositing the funds. This might introduce a lot of overhead for the calculations, an alternative solution is to have the _stake() functions always return 0 if stakeAmount is lower than a certain (small) threshold:

function _stake(uint256 stakeAmount) internal override returns (uint256) {
    if (stakeAmount < 1e6) return 0;
    ...SNIP...
}

If going for a different fix please note that the EETHAdapter will actually revert on the internal call to deposit() if 0 shares are minted, instead of in the adapter.

sherlock-admin2 commented 4 months ago

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

zzykxx commented 4 months ago

An edge case was found in the proposed fix. _stake() could still revert if RSETH_DEPOSIT_POOL.minAmountToDeposit() returns 0. This has been fixed in a new PR: https://github.com/napierfi/napier-uups-adapters/pull/23