batchDepositETHForStaking() malicious parameter to steal ETH

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantSavETHVaultPool.sol#L48-L52

Vulnerability details

Impact

GiantSavETHVaultPool#batchDepositETHForStaking() and GiantMevAndFeesPool#batchDepositETHForStaking()
No detection of _stakingFundsVault is legal, you can pass malicious _savETHVaults to steal ETH

Proof of Concept

The security check of #batchDepositETHForStaking() only detects the incoming parameters calling _savETHVaults[i].liquidStakingManager()
We can pass in a malicious contract, which has the method liquidStakingManager() to return a normal LiquidStakingManager contract address to pass the security check

    function batchDepositETHForStaking(
        address[] calldata _savETHVaults,
        uint256[] calldata _ETHTransactionAmounts,
        bytes[][] calldata _blsPublicKeys,
        uint256[][] calldata _stakeAmounts
    ) public {
...
            //***@audit The malicious savETHPool.liquidStakingManager() returns a normal liquidStakingManager to skip the security detection***//
            require(
                liquidStakingDerivativeFactory.isLiquidStakingManager(address(savETHPool.liquidStakingManager())), 
                "Invalid liquid staking manager"
            );    


            savETHPool.batchDepositETHForStaking{ value: transactionAmount }( //***@audit steal eth***//
                _blsPublicKeys[i],
                _stakeAmounts[i]
            );            

Malicious contracts such as:

contract BadETHVaults{
    
    function liquidStakingManager() external (ILiquidStakingManager) {
        return ILiquidStakingManager("0x???"); //*** normal LiquidStakingManager address***//
    }

    function batchDepositETHForStaking(bytes[] calldata _blsPublicKeyOfKnots, uint256[] calldata _amounts) external payable {
       //*** empty ***//
    }

    //** other withdraw eth function***/
}

Note: GiantMevAndFeesPool#batchDepositETHForStaking() has the same problem.

Tools Used

Recommended Mitigation Steps

contract GiantSavETHVaultPool is StakehouseAPI, GiantPoolBase {
...
    function batchDepositETHForStaking(
        address[] calldata _savETHVaults,
        uint256[] calldata _ETHTransactionAmounts,
        bytes[][] calldata _blsPublicKeys,
        uint256[][] calldata _stakeAmounts
    ) public {
...
-            require(
-                liquidStakingDerivativeFactory.isLiquidStakingManager(address(savETHPool.liquidStakingManager())),
-                "Invalid liquid staking manager"
-            );

+            LiquidStakingManager liquidStakingManager= savETHPool.liquidStakingManager();
+            require(
+                liquidStakingDerivativeFactory.isLiquidStakingManager(address(liquidStakingManager)),
+                "Invalid liquid staking manager"
+            );
+            require(
+                address(savETHPool) == address(liquidStakingManager.savETHVault()),
+                "Invalid savETHPool"
+            );

contract GiantMevAndFeesPool is ITransferHookProcessor, GiantPoolBase, SyndicateRewardsProcessor {
...
    function batchDepositETHForStaking(
        address[] calldata _stakingFundsVault,
        uint256[] calldata _ETHTransactionAmounts,
        bytes[][] calldata _blsPublicKeyOfKnots,
        uint256[][] calldata _amounts
    ) external {
...

            StakingFundsVault sfv = StakingFundsVault(payable(_stakingFundsVault[i]));
-           require(
-               liquidStakingDerivativeFactory.isLiquidStakingManager(address(sfv.liquidStakingNetworkManager())),
-               "Invalid liquid staking manager"
-           );


+           LiquidStakingManager liquidStakingManager= sfv.liquidStakingManager();
+            require(
+               liquidStakingDerivativeFactory.isLiquidStakingManager(address(liquidStakingManager)),
+               "Invalid liquid staking manager"
+            );
+            require(
+               address(sfv) == address(liquidStakingManager.stakingFundsVault()),
+               "Invalid StakingFundsVault"
+            );  


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

[c4-judge]

dmvt marked the issue as duplicate of #36

[c4-judge]

dmvt marked the issue as satisfactory

[c4-judge]

dmvt marked the issue as partial-50

[C4-Staff]

JeeberC4 marked the issue as duplicate of #36

[C4-Staff]

liveactionllama marked the issue as not a duplicate

[C4-Staff]

liveactionllama marked the issue as duplicate of #251