LSD Network - Stakehouse contest - minhtrng's results

Lines of code

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

Vulnerability details

Impact

The LP tokens have a mechanism that prevent certain interactions if some LP tokens have been recently added to the calling address. This can be used by a malicious actor to grief an address by sending tiny amounts of LP tokens to that address, either in certain intervals or by watching the mempools for transactions from that address and frontrunning those.

The features that are affected are claiming rewards, burning LPs for Ether, rotating LP tokens, and withdrawing LP tokens.

Proof of Concept

The LPToken._afterTokenTransfer hook sets the lastInteractedTimestamp for both _from and _to:

lastInteractedTimestamp[_from] = block.timestamp;
lastInteractedTimestamp[_to] = block.timestamp;

This lastInteractedTimestamp is used to restrict the execution of certain functions such as SavEthVault.burnLPToken:

bool isStaleLiquidity = _lpToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp;
...
require(isStaleLiquidity, "Liquidity is still fresh");

This allows transfers of tiny amounts to an address to prevent that address from calling functions that use the lastInteractedTimestamp in require statements.

Tools Used

Manual Review

Recommended Mitigation Steps

A workaround could be by involving a checkpointing mechanism that stores balances for a blocknumber whenever the balance changes. However that would increase implementation complexity by a fair margin.

Lines of code

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

Vulnerability details

Impact

The totalSupply of StakingFundsVault.lpTokenForKnot can be increased above the intended limit of 4 ether worth of deposits. This can be used to permanently DOS the staking of a certain blsPublicKey.

Proof of Concept

The LiquidStakingManager.stake function calls _assertEtherIsReadyForValidatorStaking(blsPubKey); to check if enough ether has been deposited in the corresponding vaults for staking. This function performs the following check:

LPToken stakingFundsLP = stakingFundsVault.lpTokenForKnot(blsPubKey);
...
require(stakingFundsLP.totalSupply() == 4 ether, "DAO staking funds vault balance must be at least 4 ether");

The ETHPoolLPFactory (which StakingFundsVault inherits from) limits the minting of LP above 4 ether by using the following check in _depositETHForStaking:

// maxStakingAmountPerValidator is set to 4 ether in StakingFundsVault._init
require(lpToken.totalSupply() + _amount <= maxStakingAmountPerValidator,

However the limit can be circumvented by using the ETHPoolLPFactory.rotateLPTokens function (which is public and inherited by StakingFundsVault):

require(_amount + _newLPToken.totalSupply() <= 24 ether, "Not enough mintable tokens");
...
_oldLPToken.burn(msg.sender, _amount);
...
_newLPToken.mint(msg.sender, _amount);

The problem is that the limit of the totalSupply is hardcoded as 24 ether, which is the limit that is intended for SavETHVault but not for StakingFundsVault as pointed out above.

Hence a malicious actor can deposit at least 0.0001 Ether once the deposits in StakingFundsVault hit 4 ether to DOS the LiquidStakingManager.stake function by using the rotateLP function and burning the LP tokens from another token contract.

To permanently DOS the staking for a specific blsPublicKey the malicious actor could also deposit more than 4 ether in total by himself. This way it would not be possible for others to withdraw deposits and redeposit the difference necessary to hit exactly 4 ether worth of deposits.

Tools Used

Manual Review

Recommended Mitigation Steps

In the function ETHPoolLPFactory.rotateLPTokens change the hardcoded 24 ether to use maxStakingAmountPerValidator instead.

Alternatively, change the check require(stakingFundsLP.totalSupply() == 4 ether from == to >=.