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

8 stars 7 forks source link

KupiaSec - EETHAdapter.claimWithdrawal() uses a wrong condition and it doesn't work in most cases #78

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

KupiaSec

medium

EETHAdapter.claimWithdrawal() uses a wrong condition and it doesn't work in most cases

Summary

EETHAdapter.claimWithdrawal() validates a wrong condition and the function doesn't work in most cases consequently.

Vulnerability Detail

EETHAdapter.claimWithdrawal() checks if _requestId is finalized on etherfi as follows:

EETHAdapter.sol
63:         if (_requestId < ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId()) revert RequestInQueue();

After that EETHAdapter.claimWithdrawal() claims withdrawal using ETHERFI_WITHDRAW_NFT.claimWithdraw(). And then ETHERFI_WITHDRAW_NFT.claimWithdraw() calls ETHERFI_WITHDRAW_NFT.getClaimableAmount() internally, and ETHERFI_WITHDRAW_NFT.getClaimableAmount() also checks if the request id is finalized.

WithdrawRequestNFT.sol
87:    function claimWithdraw(uint256 tokenId) public {

92:        uint256 amountToWithdraw = getClaimableAmount(tokenId);
WithdrawRequestNFT.sol

68:    function getClaimableAmount(uint256 tokenId) public view returns (uint256) {

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

As we can see from here, the finalized token id should be equal or less than lastFinalizedRequestId. So the condition used in EETHAdapter.claimWithdrawal() is not correct. As a result, EETHAdapter.claimWithdrawal() works only when _requestId = ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId, and this is not the intended behavior.

Impact

EETHAdapter.claimWithdrawal() works only when _requestId = ETHERFI_WITHDRAW_NFT.lastFinalizedRequestId, and doesn't work in most cases.

Tool used

Manual Review

Code Snippet

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

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

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

Recommendation

The _requestId should be larger than lastFinalizedRequestId.

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

Duplicate of #55