LSD Network - Stakehouse contest - arcoun's results

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L63

Vulnerability details

Impact

In the _distributeETHRewardsToUserForToken() function, the claimed array is updated with the claimed rewards of each user. This array is also used to calculate how much awards a user can withdraw.

There is an error when the array is updated: instead of increasing the value with the withdrawn awards (due), the value is replaced with the withdrawn awards (due). The new value will then be lower than the expected one, allowing a user to withdraw more than due by withdrawing several times.

uint256 due = ((accumulatedETHPerLPShare * balance) / PRECISION) - claimed[_user][_token];
if (due > 0) {
    claimed[_user][_token] = due;   // should be "+= due"
    [...]
}

The issue can also prevent a user to interact with the contract and withdraw his own assets because _distributeETHRewardsToUserForToken() will revert if the calculated due is higher than the ETH balance in the contract.

Proof of Concept

Proof of concept using GiantMevAndFeesPool.claimRewards() but other paths are available.

• A user has a positive lpTokenETH balance on the GiantMevAndFeesPool contract, allowing him to withdraw awards
• The user call the claimRewards() function on the GiantMevAndFeesPool contract with the following parameters:
  • _recipient: user's address
  • _stakingFundsVaults: array with at least one available stacking fund. Note that the user can also provide an alternate contract which does not revert on claimRewards(address,bytes[]) as there is no verification here
  • _blsPublicKeysForKnots: related BLS keys (if using an available stacking fund)
• After the call, due rewards have been sent to the user and claimed[user][lpTokenEth] is now set to a lower value than expected
• When calling claimRewards() at a later time, due rewards will be miscalculated and be higher than expected
  • If there is enough ETH balance at that time, the related Ether will be sent to user
  • If there is not enough ETH balance, the transaction will revert. As _distributeETHRewardsToUserForToken() is called at other places in the contract, this will prevent the user to do some actions like withdrawing his own assets

Tools Used

Manual review.

Recommended Mitigation Steps

Fix the _distributeETHRewardsToUserForToken() function to increase claimed[_user][_token] with due instead of updating it with due.

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantSavETHVaultPool.sol#L48-L58 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantMevAndFeesPool.sol#L42-L51

Vulnerability details

Impact

Due to a lack of validation of input parameters in batchDepositETHForStaking(), it is possible to use an alternate contract as a vault and withdraw all available Ether from the contract. The issue is present in both GiantSavETHVaultPool and GiantMevAndFeesPool contracts.

The only validation done is on a value returned by the user-provided address, this can be easily defeated by implementing the related funtion and returning a valid value.

The function will then call the batchDepositETHForStaking() function on the user-provided address, with the requested Ether amount. If the address implements this function as a payable function, Ether will be transfered to it.

Proof of Concept

• Create a dedicated contract to exploit the vulnerability
  • Implement liquidStakingManager() and return the address of a valid Liquid Staking Manager when called
  • Implement liquidStakingNetworkManager() and return the address of a valid Liquid Staking Manager for when called
  • Implement batchDepositETHForStaking(bytes[],uint256[]) as a payable function to not throw when called with an Ether value
  • Implement a function to safely withdraw any Ether from the contract
• Call GiantSavETHVaultPool.batchDepositETHForStaking() with the following parameters
  • _savETHVaults: array of size 1 with the created contract
  • _ETHTransactionAmounts: array of size 1 with available Ether (using idleETH())
  • _blsPublicKeys: array of size 1 with any byte[] value
  • _stakeAmounts: array of size 1 with any uint256[] value
  • The contract will transfer the request amount of Ether to the user-provided contract
• Call GiantMevAndFeesPool.batchDepositETHForStaking() with the following parameters
  • _stakingFundsVault: array of size 1 with the created contract
  • _ETHTransactionAmounts: array of size 1 with available Ether (using idleETH())
  • _blsPublicKeyOfKnots: array of size 1 with any byte[] value
  • _amounts: array of size 1 with any uint256[] value
  • The contract will transfer the request amount of Ether to the user-provided contract
• Withdraw all Ether from the created contract

Tools Used

Manual review.

Recommended Mitigation Steps

User-provided contract addresses must be checked to avoid a malicious usage.

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantPoolBase.sol#L82-L86

Vulnerability details

Impact

Due to a lack of validation of input parameters in GiantPoolBase.withdrawLPTokens(), it is possible to exchange Giant LP tokens with any ERC20 token owned by the target contract, not only Vault LP token as expected by the function. This issue impacts any contract inherited from GiantPoolBase: GiantMevAndFeesPool and GiantSavETHVaultPool.

During its lifetime, a contract can receive various ERC20 tokens: airdrops, user error, gifts, spam, ... Some of them may have a tradable value and should be kept by the operators of the contract.

If the raw value (as uint256 value, without decimal) of a token is greater than the raw value of the Giant LP token, a user may abuse the lack of validation of withdrawLPTokens() to exchange them and make a profit.

Limitations:

• At least MIN_STAKING_AMOUNT (0.001 ether) must be exchanged
• On GiantMevAndFeesPool, target token must support self-transfer as done in _onWithdraw() (GiantMevAndFeesPool.sol#L184). Most ERC tokens support that.

Proof of Concept

• The GiantMevAndFeesPool or GiantSavETHVaultPool contract owns 1000 FOO tokens. This is an example token with decimals=12 and a value of $5 per token ($5000 for the 1000 FOO tokens).
• The (raw) balance of the contract for FOO is 1000000000000000 (1000*10^12)
• A user deposits 0.001 Ether (MIN_STAKING_AMOUNT) using the depositETH() function and now have 0.001 Giant LP Tokens (0.001*10^18 = 1000000000000000)
• The user can now exchange his Giant LP Tokens for the FOO tokens
• The user call withdrawLPTokens() with the following parameters:
  • _lpTokens: array of size 1 with the FOO token address
  • _amounts: array of size 1 with the MIN_STAKING_AMOUNT amount
• During the call, the 0.001 Giant LP tokens ($1.2) of the user are burnt and the 1000 FOO tokens ($5000) are transfered to the user

Tools Used

Manual review.

Recommended Mitigation Steps

User-provided token addresses must be checked to avoid a malicious usage.