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

8 stars 7 forks source link

Drynooo - Funds may not be withdrawn due to a check error #61

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 5 months ago

Drynooo

high

Funds may not be withdrawn due to a check error

Summary

In the claimWithdrawal function, the contract will check _requestId, but the error in judgment here means that unless _requestId == ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId(), the protocol will never be able to claim.

Vulnerability Detail

The following judgments are made in the contract

        if (_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();

This judgment means that if the latest finalized id is larger than _requestId, revet.

This is exactly the opposite of the meaning of ETHERFI_WITHDRAW_NFT. The judgment in ETHERFI_WITHDRAW_NFT is as follows

        require(tokenId <= lastFinalizedRequestId, "Request is not finalized");

Therefore, the error in judgment here causes the withdrawal to be locked once the opportunity for _requestId == ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId() is missed. This is highly likely to happen.

Impact

The protocol may not be able to withdraw funds resulting in loss of funds.

Code Snippet

https://github.com/sherlock-audit/2024-05-napier-update/blob/c31af59c6399182fd04b40530d79d98632d2bfa7/napier-v1/src/adapters/etherfi/EETHAdapter.sol#L63 https://www.codeslaw.app/contracts/ethereum/0xdaaac9488f9934956b55fcdaef6f9d92f8008ca7?file=src%2FWithdrawRequestNFT.sol&start=70

Tool used

Manual Review

Recommendation

-        if (_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();
+        if (_requestId > ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();

Duplicate of #55