[Security] WBERAStakerVault: completeWithdrawal(bool, uint256) has no caller ownership check

[sailorpepe]

Summary

completeWithdrawal(bool isNative, uint256 requestId) allows any address to complete any user's ERC721-based withdrawal request. The underlying _burnWithdrawalRequest in WBERAStakerVaultWithdrawalRequest calls _burn(requestId) which does not verify the caller is the NFT owner.

Impact

Low — No fund theft is possible because assets are sent to request.receiver (set at queue time, not controlled by the caller). However:

• An attacker can force-complete someone else's withdrawal before the user is ready
• The attacker chooses the isNative flag, potentially sending native BERA instead of WBERA (or vice versa)

Note: cancelQueuedWithdrawal correctly checks NFT ownership (line 240), but completeWithdrawal does not.

Affected Code

• WBERAStakerVault.sol#L263-L266 — No ownership check
• WBERAStakerVaultWithdrawalRequest.sol#L161-L171 — _burn does not check caller

Compare with cancelQueuedWithdrawal which does check ownership:

if (msg.sender != IERC721(address(withdrawalRequests721)).ownerOf(requestId)) {
    OnlyNFTOwnerAllowed.selector.revertWith();
}

Recommended Fix

Add the same ownership check to completeWithdrawal:

function completeWithdrawal(bool isNative, uint256 requestId) external nonReentrant whenNotPaused {
    if (msg.sender != IERC721(address(withdrawalRequests721)).ownerOf(requestId)) {
        OnlyNFTOwnerAllowed.selector.revertWith();
    }
    _completeWithdrawal(isNative, requestId);
}

[sailorpepe]

Additional Finding: LSTStakerVault has the same issue

LSTStakerVault.completeWithdrawal (line 188 of src/pol/lst/LSTStakerVault.sol) also has no caller ownership check:

function completeWithdrawal(uint256 requestId) external nonReentrant whenNotPaused {
    _completeWithdrawal(requestId);
}

The underlying LSTStakerVaultWithdrawalRequest._burnWithdrawalRequest (line 165) uses _burn(requestId) with no auth check — same pattern as WBERAStakerVaultWithdrawalRequest.

Meanwhile, LSTStakerVault.cancelQueuedWithdrawal (line 166) does check:

if (msg.sender != IERC721(address(withdrawalRequests721)).ownerOf(requestId)) {
    OnlyNFTOwnerAllowed.selector.revertWith();
}

This confirms the missing ownership check is a pattern-level bug across both vault types, not a one-off oversight.

Affected files:

• LSTStakerVault.sol#L188
• LSTStakerVaultWithdrawalRequest.sol#L165

Recommended fix — same as WBERAStakerVault:

function completeWithdrawal(uint256 requestId) external nonReentrant whenNotPaused {
    if (msg.sender != IERC721(address(withdrawalRequests721)).ownerOf(requestId)) {
        OnlyNFTOwnerAllowed.selector.revertWith();
    }
    _completeWithdrawal(requestId);
}