LSD Network - Stakehouse contest - wait's results

Lines of code

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

Vulnerability details

Impact

Attacker can take away all eth in the pools.

Proof of Concept

Both GiantSavETHVaultPool and GiantMevAndFeesPool provide a function - batchDepositETHForStaking to deposit ETH from pool to vault for staking.

The main logic is:

for (uint256 i; i < numOfSavETHVaults; ++i) {
    uint256 transactionAmount = _ETHTransactionAmounts[i];

    // As ETH is being deployed to a savETH pool vault, it is no longer idle
    idleETH -= transactionAmount;

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

    // Deposit ETH for staking of BLS key
    savETHPool.batchDepositETHForStaking{ value: transactionAmount }(
        _blsPublicKeys[i],
        _stakeAmounts[i]
    );
}

It will check the vault by the liquidStakingManager address, and then send ETH to the vault by calling batchDepositETHForStaking.

Attacker can deploy a mock vault contract (M) which implements following two functions:

1. function isLiquidStakingManager() returns the correct liquidStakingManager address.
2. functoin batchDepositETHForStaking(bytes[], uint256[]) payable to receive ETH.

Then the attacker can call batchDepositETHForStaking of the pools with the mock vault contract M, and ETH in pools will be sent to M directly.

Tools Used

n/a

Recommended Mitigation Steps

We need check the vault addresses passed to batchDepositETHForStaking, not to check the liquidStakingManager returned by the vault contract.

An easy way to do this is to register all newly created valid vault contracts in a trusted contract like the liquidStakingDerivativeFactory(LSDNFactory), and then check within the batchDepositETHForStaking to see if vault is in the registry.

Lines of code

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

Vulnerability details

Impact

Anyone can exchange ETH for any token in the pool at a ratio of 1:1

Proof of Concept

The main logic of withdrawLPTokens function in GiantPoolBase is:

    // Burn giant LP from user before sending them an LP token from this pool
    lpTokenETH.burn(msg.sender, amount);

    // Giant LP tokens in this pool are 1:1 exchangeable with external savETH vault LP
    token.transfer(msg.sender, amount);

The function doesn't check what the token passed in is, and calls token.transfer(msg.sender, amount); directly after eth lp is burnt.

If there is a token T with higher price than eth in the pool, then an attacker can pass in the address of the token T as a lpToken to swap ETH for T.

Tools Used

n/a

Recommended Mitigation Steps

Register all LPTokens in a global trusted contract (e.g. LSDNFactory - liquidStakingDerivativeFactory), and check the passed in lpToken addresses in the registry.