aman - `WithdrawRequestBase:_splitWithdrawRequest` assigns a request ID of `0` to `_to` when `w.vaultShares == vaultShares` and the vault shares cannot be redeemed.

[sherlock-admin2]

aman

High

WithdrawRequestBase:_splitWithdrawRequest assigns a request ID of 0 to _to when w.vaultShares == vaultShares and the vault shares cannot be redeemed.

Summary

While liquidating a user, the protocol checks for any active requests from the target account and assigns that request to the liquidator. However, if w.vaultshare == valueShare, the request is deleted. Despite this deletion, the protocol subsequently assigns the deleted request ID to the liquidator during further execution.

Vulnerability Detail

When user is liquidated the Protocol will calls BaseStakingVault:deleverageAccount function , This function will calls the _splitWithdrawRequest function which will gives the current requestID, if exist, to liquidator. The Issue arises in _splitWithdrawRequest when the request of _from address got deleted.

    function _splitWithdrawRequest(address _from, address _to, uint256 vaultShares) internal {
        WithdrawRequest storage w = VaultStorage.getAccountWithdrawRequest()[_from];
        if (w.requestId == 0) return;

        // Create a new split withdraw request
        if (!w.hasSplit) {
            SplitWithdrawRequest memory s = VaultStorage.getSplitWithdrawRequest()[w.requestId];
            // Safety check to ensure that the split withdraw request is not active, split withdraw
            // requests are never deleted. This presumes that all withdraw request ids are unique.
            require(s.finalized == false && s.totalVaultShares == 0);
            VaultStorage.getSplitWithdrawRequest()[w.requestId].totalVaultShares = w.vaultShares;
        }

 @1>--       if (w.vaultShares == vaultShares) {
            // If the resulting vault shares is zero, then delete the request. The _from account's
            // withdraw request is fully transferred to _to
 @2>--           delete VaultStorage.getAccountWithdrawRequest()[_from];
        } else {
            // Otherwise deduct the vault shares
            w.vaultShares = w.vaultShares - vaultShares;
            w.hasSplit = true;
        }

        // Ensure that no withdraw request gets overridden, the _to account always receives their withdraw
        // request in the account withdraw slot.
        WithdrawRequest storage toWithdraw = VaultStorage.getAccountWithdrawRequest()[_to];
        require(toWithdraw.requestId == 0 || toWithdraw.requestId == w.requestId , "Existing Request");

        // Either the request gets set or it gets incremented here.
@3>--    toWithdraw.requestId = w.requestId; // @audit : the request id will be zero here
        toWithdraw.vaultShares = toWithdraw.vaultShares + vaultShares;
        toWithdraw.hasSplit = true;
    }
}

Now let have look how withdraw requests are finalized: To finilze withdraw request the National will calls _redeemFromNotional function :

 function _redeemFromNotional(
        address account,
        uint256 vaultShares,
        uint256 maturity,
        bytes calldata data
    ) internal override returns (uint256 borrowedCurrencyAmount) {
        // Short circuit here to allow for direct repayment of debts. This method always
        // gets called by Notional on every exit, but in times of illiquidity an account
        // may want to pay down their debt without being able to instantly redeem their
        // vault shares to avoid liquidation.
        if (vaultShares == 0) return 0;

        WithdrawRequest memory accountWithdraw = getWithdrawRequest(account);

        RedeemParams memory params = abi.decode(data, (RedeemParams));
 @1>--       if (accountWithdraw.requestId == 0) {
            return _executeInstantRedemption(account, vaultShares, maturity, params);
        } else {
            (
                uint256 vaultSharesRedeemed,
                uint256 tokensClaimed
            ) = _redeemActiveWithdrawRequest(account, accountWithdraw);
            // Once a withdraw request is initiated, the full amount must be redeemed from the vault.
            require(vaultShares == vaultSharesRedeemed);
            ...

The requestId for account is 0 it will calls the _executeInstantRedemption instead of calling _redeemActiveWithdrawRequest.

The Following case would occur:

1. The _from address has active request with w.vaultShare =12e18;
2. vaultShares = 12e18 so here w.vaultshare==vaultShare.
3. We delete the Request from storage at @2>-- which will make the w.requestId==0.
4. The _to address have no active withdraw request. we will assign the current request to _to address.
5. As we know that the w.requestId=0, there for toWithdraw.requestId=0.
6. The Liquidator will not be able to withdraw vaultShare because inside _redeemFromNotional function we check requestId!=0 then call _redeemActiveWithdrawRequest function.

Due to time constraint I was not able to write a code POC but I will share simple POC which will demonstrate that the requestId will be zero here.

POC

Add Following file to test suite: ```solidity // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.22; import "forge-std/Test.sol"; contract TestRequestID is Test { struct WithdrawRequest { uint256 requestId; bool hasSplit; uint256 vaultShare; } mapping(uint256 => WithdrawRequest) public requests; function setUp() external { requests[1] = WithdrawRequest({requestId: 1, hasSplit: true , vaultShare:120}); } function testDeletedRequestId() external { WithdrawRequest storage w = requests[1]; delete requests[1]; console.log("requestId" , w.requestId); assertEq(0 , w.requestId); } } ``` run with the command : `forge test --mt testDeletedRequestId -vvv` output : ```javascript [PASS] testDeletedRequestId() (gas: 12956) Logs: requestId 0 ```

Impact

The Liquidator will not be able to withdraw the VaultShare which he received via Liquidation.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/main/leveraged-vaults-private/contracts/vaults/common/WithdrawRequestBase.sol#L205-L238 https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/main/leveraged-vaults-private/contracts/vaults/staking/BaseStakingVault.sol#L148

Tool used

Manual Review

Recommendation

Either Cache the RequestId or delete the request at the end of _splitWithdrawRequest function.

Duplicate of #6

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

w has been assigned and will not be reset