LSD Network - Stakehouse contest - bin2chen's results

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]
            );
        }
    }

Lines of code

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

Vulnerability details

Impact

GiantSavETHVaultPool#bringUnusedETHBackIntoGiantPool() burn lpToken to send eth back,but GiantSavETHVaultPool no #receive() method to accept eth, will fail

Proof of Concept

GiantSavETHVaultPool#bringUnusedETHBackIntoGiantPool() -> SavETHVault#burnLPToken() -> msg.sender.call{value: _amount}("") but SavETHVault no receive()

contract SavETHVault is Initializable, ETHPoolLPFactory, ReentrancyGuard {
...
    function burnLPToken(LPToken _lpToken, uint256 _amount) public nonReentrant returns (uint256) {
        (bool result,) = msg.sender.call{value: _amount}(""); //****audit GiantSavETHVaultPool  need #receive() to accept eth
        require(result, "Transfer failed");
    }

Tools Used

Recommended Mitigation Steps

contract GiantSavETHVaultPool is StakehouseAPI, GiantPoolBase {
...

+    /// @notice Allow the contract to receive ETH
+    receive() external payable {}

}

Lines of code

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

Vulnerability details

Impact

ETHPoolLPFactory#rotateLPTokens() Wrong use of "24 ether", should use variable maxStakingAmountPerValidator, resulting in StakingFundsVault's lpToken.totalSupply() may be greater than 4 eth

Proof of Concept

use "24 eth " ,must use maxStakingAmountPerValidator

abstract contract ETHPoolLPFactory is StakehouseAPI {

    function rotateLPTokens(LPToken _oldLPToken, LPToken _newLPToken, uint256 _amount) public {
...
        require(_amount + _newLPToken.totalSupply() <= 24 ether, "Not enough mintable tokens"); //***@audit use 24 eth ,must use  maxStakingAmountPerValidator***//   

contract StakingFundsVault is
    Initializable, ITransferHookProcessor, StakehouseAPI, ETHPoolLPFactory, SyndicateRewardsProcessor, ReentrancyGuard
{
...

    function _init(LiquidStakingManager _liquidStakingNetworkManager, LPTokenFactory _lpTokenFactory) internal virtual {
....
        maxStakingAmountPerValidator = 4 ether; //***@auit  maxStakingAmountPerValidator = 4 eth ,not 24 eth**/
    }

Tools Used

Recommended Mitigation Steps

abstract contract ETHPoolLPFactory is StakehouseAPI {

    function rotateLPTokens(LPToken _oldLPToken, LPToken _newLPToken, uint256 _amount) public {
...
-        require(_amount + _newLPToken.totalSupply() <= 24 ether, "Not enough mintable tokens");
+        require(_amount + _newLPToken.totalSupply() <= maxStakingAmountPerValidator, "Not enough mintable tokens");