whitehair0330 - Invalid check `_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()` in the `EETHAdapter.claimWithdrawal()` function.

[sherlock-admin3]

whitehair0330

high

Invalid check _requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() in the EETHAdapter.claimWithdrawal() function.

Summary

ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() returns the last requestId which is finalized withdrawl. So, _requestId > ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() means that the _reuestId is not finalized yet. However, the EETHAdapter.claimWithdrawal() function reverts when _requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId(). So, some ethers will be frozen in the LiquidityPool of the EtherFi protocol.

Vulnerability Detail

ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() returns the last _requestId which is finalized withdrawl. And, _requestId > ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() means that the _reuestId is not finalized yet.

So, in the following codes of the EtherFi protocol, tokenId <= lastFinalizedRequestId is required when claiming withdraw .

https://github.com/etherfi-protocol/smart-contracts/blob/master/src/WithdrawRequestNFT.sol#L87-L111

    function claimWithdraw(uint256 tokenId) public {
            [...]
@>      uint256 amountToWithdraw = getClaimableAmount(tokenId);
            [...]
    }

https://github.com/etherfi-protocol/smart-contracts/blob/master/src/WithdrawRequestNFT.sol#L68-L82

    function getClaimableAmount(uint256 tokenId) public view returns (uint256) {
        require(tokenId < nextRequestId, "Request does not exist");
@>      require(tokenId <= lastFinalizedRequestId, "Request is not finalized");
            [...]
    }

However, the EETHAdapter.claimWithdrawal() function reverts when _requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId().

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

    function claimWithdrawal(uint256 _requestId) external override nonReentrant {
        uint256 queued = queueWithdrawal[_requestId];
        if (queued == 0) revert NoPendingWithdrawal();

        /// ASSERT ///
        // EtherFi is completing withdraws internally and its number is set to lastFinalizedRequestId.
        // If _requstId is finalized on etherfi, it's reverted.
@>      if (_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();

        /// INTERACT ///
        // Claimed amount can be less than requested amount due to slashing.
        uint256 balanceBefore = address(this).balance;
        ETHERFI_WITHDRAW_NFT.claimWithdraw(_requestId);
        uint256 claimed = address(this).balance - balanceBefore;
        /// WRITE ///
        delete queueWithdrawal[_requestId];
        totalQueueEth -= queued.toUint128();
        bufferEth += claimed.toUint128();

        IWETH9(Constants.WETH).deposit{value: claimed}();
        emit ClaimWithdrawal(_requestId, claimed);
    }

As a result, withdrawl claiming will always reverts when _requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId(). It is very possible in the EtherFi protocol. It means that the ether will be frozen in the EtherFi protocol, because withdraw claim of the EtherFi protocol cannot be called.

Impact

Some ethers will be frozen in the LiquidityPool of the EtherFi protocol.

Code Snippet

https://github.com/etherfi-protocol/smart-contracts/blob/master/src/WithdrawRequestNFT.sol#L87-L111

https://github.com/etherfi-protocol/smart-contracts/blob/master/src/WithdrawRequestNFT.sol#L68-L82

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

Tool used

Manual Review

Recommendation

_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() should be removed, or be replaced by _requestId > ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId().

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

    function claimWithdrawal(uint256 _requestId) external override nonReentrant {
        uint256 queued = queueWithdrawal[_requestId];
        if (queued == 0) revert NoPendingWithdrawal();

        /// ASSERT ///
        // EtherFi is completing withdraws internally and its number is set to lastFinalizedRequestId.
        // If _requstId is finalized on etherfi, it's reverted.
-       if (_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();
+       if (_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();

        /// INTERACT ///
        // Claimed amount can be less than requested amount due to slashing.
        uint256 balanceBefore = address(this).balance;
        ETHERFI_WITHDRAW_NFT.claimWithdraw(_requestId);
        uint256 claimed = address(this).balance - balanceBefore;
        /// WRITE ///
        delete queueWithdrawal[_requestId];
        totalQueueEth -= queued.toUint128();
        bufferEth += claimed.toUint128();

        IWETH9(Constants.WETH).deposit{value: claimed}();
        emit ClaimWithdrawal(_requestId, claimed);
    }

Duplicate of #55