batchDepositETHForStaking in GiantSavETHVaultPool.sol can be ticked to steal all ETH in the pool

[code423n4]

Lines of code

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

Vulnerability details

Impact

All Eth can be drained by fake vault addresses.
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantSavETHVaultPool.sol#L29

Proof of Concept

In batchDepositETHForStaking, _savETHVault is checked for its validity through

SavETHVault savETHPool = SavETHVault(_savETHVaults[i]); 
require(
    liquidStakingDerivativeFactory.isLiquidStakingManager(address(savETHPool.liquidStakingManager())),
    "Invalid liquid staking manager"
);

However, an attacker can create a fake contract that retuns a correct liquidStakingNetworkManager, thus passing the check easily.
After the check, any ETH in the pool will be sent to an address this fake contract provide:

savETHPool.batchDepositETHForStaking{ value: transactionAmount }(
    _blsPublicKeys[i],
    _stakeAmounts[i]
);

Tools Used

manual

Recommended Mitigation Steps

Always passing liquid staking manager address, checking its real and then requesting either the savETH vault or staking funds vault is a good idea rather than other way around from a giant pool perspective.

[c4-judge]

dmvt marked the issue as duplicate of #36

[c4-judge]

dmvt marked the issue as satisfactory

[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