Bluedragon - MultiSig Wallets Can't Receive Native ETH Due To The `transfer()` Function Gas Constraint Causing Variable Users Unable to Receive Their Pending Variable Amount.

[sherlock-admin2]

Bluedragon

Medium

MultiSig Wallets Can't Receive Native ETH Due To The transfer() Function Gas Constraint Causing Variable Users Unable to Receive Their Pending Variable Amount.

Summary:

In LidoVault contract, variable users can get back their variable amount on an ongoing vault. However when fee receiver finalizes an variable user's ongoing withdrawal requests using feeReceiverFinalizeVaultOngoingVariableWithdrawals function, their variable users withdrawal amount is updated in variableToPendingWithdrawalAmount mapping, which can be later withdrawn by the variable user using withdrawAmountVariablePending function. However, the withdrawAmountVariablePending function uses transfer() function to send the variable amount to the variable user, which can fail if the user uses a multiSig wallet due to the gas constraint. This can cause the variable user to be unable to receive their pending variable amount.

Vulnerability Details:

The transfer function is a low-level call that automatically forwards 2300 gas to the recipient (msg.sender) when transferring native tokens (e.g., ETH). However, transfer() only forwards 2300 gas, which is not enough for the recipient to execute any non-trivial logic in a receive() or fallback function. For instance, it is not enough for Safes (such as this one) to receive funds, which require more than 6k gas for the call to reach the implementation contract and emit an event.

Impact:

Variable users of the LidoVault contract who use multiSig wallets will be unable to receive their pending variable amount, as the transfer() function will fail in withdrawAmountVariablePending function due to the gas constraint. This can lead to a loss of funds for the variable users.

Code Snippet:

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L653-L657

Proof Of Concept:

1. Variable user deposits into the Lido vault for the yeild generated from the fixed users captial.
2. Variable user requests a withdrawal of their variable amount.
3. Fee receiver finalizes the variable user's withdrawal request using feeReceiverFinalizeVaultOngoingVariableWithdrawals function.
4. Variable user tries to withdraw their pending variable amount using withdrawAmountVariablePending function.
5. The transfer() function fails due to the gas constraint, causing the variable user to be unable to receive their pending variable amount.
6. The variable user loses their funds.

Proof Of Code:

To run the test, follow the steps below:

1. Modify the uint256 lidosEthlidoStETHBalance variable in the withdraw function to ⬇️

-   lidostEthlidoStETHBalance = stakingBalance();
    // @info this modified to simulate a rebasing effect
+   lidostEthlidoStETHBalance = stakingBalance() + 20 ether;

1. Modify the uint256 withdrawnAmount variable in _claimWithdrawals function to ⬇️

-    uint256 withdrawnAmount = address(this).balance - beforeBalance;
    // @info this modified to simulate a eth is received from lido
+   uint256 withdrawnAmount = 20 ether;

1. Run the test using the following code:

    forge test --mt test_MultiSigVariableUsersWithdraw -vvvv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import {Test, console, Vm} from "forge-std/Test.sol";
import {VaultFactory} from "../src/VaultFactory.sol";
import {LidoVault} from "../src/LidoVault.sol";
import {ILidoWithdrawalQueueERC721} from "../src/interfaces/ILidoWithdrawalQueueERC721.sol";

contract BugTest is Test {
    VaultFactory public factory;
    LidoVault public vault;
    address owner = makeAddr("owner");
    address alice = address(0x123);
    address bob = address(0x456);
    address charlie = address(0x789);
    address vaultAddr;
    uint256 fixedSideCapacity = 100 ether;
    uint256 variableSideCapacity = 30 ether;
    uint256 duration = 30 days;

    function setUp() public {
        vm.createSelectFork("https://rpc.ankr.com/eth");
        uint256 protocolFeeBps = 100;
        uint256 earlyExitFeeBps = 1000;
        vm.startPrank(owner);
        factory = new VaultFactory(protocolFeeBps, earlyExitFeeBps);
        // @info Setting the protocol fee receiver
        factory.setProtocolFeeReceiver(owner);
        // @info Creating a vault
        factory.createVault(fixedSideCapacity, duration, variableSideCapacity);
        (, vaultAddr) = factory.vaultInfo(1);
        vault = LidoVault(payable(vaultAddr));
        vm.stopPrank();
    }

        function test_MultiSigVariableUsersWithdraw() public {
        MultiSigWallet multiSig = new MultiSigWallet();
        address multiSigAddr = address(multiSig);
        uint256 amount = 50 ether;
        deal(alice, 100 ether);
        deal(multiSigAddr, 15 ether);
        deal(charlie, 15 ether);
        // @info Alice deposited in Fixed side
        vm.startPrank(alice);
        vault.deposit{value: amount * 2}(0);
        vm.stopPrank();

        // @info Bob deposited in Variable side
        vm.startPrank(multiSigAddr);
        vault.deposit{value: 15 ether}(1);
        vm.stopPrank();

        // @info Charlie deposited in Variable side
        vm.startPrank(charlie);
        vault.deposit{value: 15 ether}(1);
        vm.stopPrank();

        // @info Bob Withdraws in the middle of the vault
        vm.warp(block.timestamp + 1 days);
        vm.startPrank(multiSigAddr);
        vault.withdraw(1);
        vm.stopPrank();

        uint256[] memory requestId = vault.getVariableToVaultOngoingWithdrawalRequestIds(multiSigAddr);
        // @info mock call simulating claim withdrawal
        mockClaimWithdrawal(requestId[0], 120 ether);

        // @info Fee receiver finalizes variable side withdrawals
        vm.startPrank(owner);
        vault.feeReceiverFinalizeVaultOngoingVariableWithdrawals(multiSigAddr);
        vm.stopPrank();

        vm.startPrank(multiSigAddr);
        vm.expectRevert("NO_GAS");
        // @info TX fails due to gas constraint
        vault.withdrawAmountVariablePending();
        vm.stopPrank();
    }

    // @info This function is created to simulate the claim withdrawal function in lido
    // as finalization of rebasing in fork testing is not possible
    function mockClaimWithdrawal(uint256 requestId, uint256 returnValue) internal {
        vm.mockCall(
        address(vault.lidoWithdrawalQueue()),
        abi.encodeWithSelector(ILidoWithdrawalQueueERC721.claimWithdrawal.selector, requestId),
        abi.encode(returnValue)
        );
        deal(address(this), 1000 ether);
        vm.prank(address(this));
        payable(address(vault)).transfer(100 ether);
    }
}

contract MultiSigWallet {
    receive() external payable {
        require(gasleft() > 6000, "NO_GAS");
    }
}

Tool Used:

• Foundry
• Manual Analysis

Recommendation:

To prevent this scenario, the LidoVault contract should use a different method to send the variable amount to the variable user, such as call(). Here is the recommended mitigation:

function withdrawAmountVariablePending() public {
    uint256 amount = variableToPendingWithdrawalAmount[msg.sender];
    variableToPendingWithdrawalAmount[msg.sender] = 0;
    // @audit use .call instead of transfer, multisg users cannot withdraw
-   payable(msg.sender).transfer(amount);
+   (bool success, ) = msg.sender.call{value: amount}("");
+   require(success, "Transfer failed");
}