Description

The storage variable lastFinalizedRequestId records the last claimed request. And the function prepareWithdrawal is used to process unclaimed requests. Thus, this function should not process a request when its _requestId equals the lastFinalizedRequestId.

function prepareWithdrawal(uint256 _requestId, uint256 _avaliableAssets)
    external
    onlyOwner
    returns (address recipient, uint256 amount, uint256 avaliableAssets)
{
    if (_requestId == 0) revert InvalidRequestId(_requestId);
!   if (_requestId < lastFinalizedRequestId) revert RequestNotFoundOrFinalized(_requestId);

    WithdrawalRequest storage request = _requests[_requestId];

    if (request.claimed) revert RequestAlreadyClaimed(_requestId);

    // [...]
}

Impact

If each request can be correctly marked as claimed or not, not checking whether the _requestId equals lastFinalizedRequestId will not have much impact — it would only cause the transaction to revert a bit later.

However, the vault has a function claimWithdrawal, which can claim any withdrawal request with the provided _requestId. Although it is currently a private function, if it were to be made public in the future and there exists a request that was incorrectly finalized but not yet marked as claimed (see Finding ref↗ for details), this request could be executed again through the function prepareWithdrawal. As a result, the receiver would receive more assets than intended, due to the combination of assets sent during the erroneous finalization and the re-execution.

Recommendations

Consider updating the check according to the following code:

-if (_requestId < lastFinalizedRequestId) revert RequestNotFoundOrFinalized(_requestId);
+if (_requestId <= lastFinalizedRequestId) revert RequestNotFoundOrFinalized(_requestId);

Remediation

This issue has been acknowledged by Blueprint Finance, and a fix was implemented in commit 574ea328↗.