Asymmetry contest - Koolex's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L72 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L94

Vulnerability details

Impact

To stake ETH into safETH, one should call SafEth.stake() method. The calculation of preDepositPrice is done like this:

1. For the first staking, the price is 1
2. For any next staking, the price is calculated as follows:

preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

Please note that the underlyingValue is the total value in ETH of all derivatives. Each derivative's underlyingValue is calculated as follows:

underlyingValue += (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;

And the derivative balance is calculated as follows (an example for WST_ETH):

function balance() public view returns (uint256) {
        return IERC20(WST_ETH).balanceOf(address(this));
}

An early malicious actor can send the derivative token directly to the derivative contract (e.g. WstEth), manipulating the underlyingValue calculation. Thus, manipulating the preDepositPrice which could possibly cause DoS to staking. Meaning that stakers can not stake any longer after the attack.

Proof of Concept

Given: Alice likes to join Asymmetry as a staker Bob is an early first staker (a malicious actor) For simplicity, safETH has only one derivative (e.g. WstEth) The price of 1 WST_ETH token = 1 ETH

Imagine the following scenario:

1. Bob stakes 1 ETH as a first staker.
2. Since the initial price is 1, Bob receives 1 safETH token.

preDepositPrice = 10 ** 18; // initializes with a price of 1

3. The derivative contract now has a balance 1 WST_ETH token
4. SafEth contract has totalSupply = 1
5. Bob buys 200 WST_ETH token from a secondry market/pool, and sends it directly to the derivative contract WstEth.
6. The derivative contract now has a balance 201 WST_ETH token.
7. Alice stakes 10 ETH
8. Alice recevies 0 safETH token. Here is why:

// Calculate underlyingValue
underlyingValue = WST_ETH price in ETH * WstEth derivative balance  / 10 ** 18
underlyingValue = 1 ETH * 201 WST_ETH token / 10 ** 18
underlyingValue = 201 ETH

// Calculate preDepositPrice
preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply
preDepositPrice = (10 ** 18 * 201) / 1 safETH token
preDepositPrice = 201 ETH

// Calculate mint amount
mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice

=> Alice staked 10, so totalStakeValueEth = 10
mintAmount = (10 * 10 ** 18) / 201 ETH
mintAmount = 0

9. Let's say Alice staked the maximum allowed which is 200 ETH
10. Do the calculation

=> Alice staked 200, so totalStakeValueEth = 200
mintAmount = (200 * 10 ** 18) / 201 ETH
mintAmount = 0

11. As you can see, Alice can not receive any safETH token from staking even if the max allowed is used.
12. Alice has to stake 201 ETH to receive 1 safETH token. However, this is not possible since it exceeds the max allowed

	maxAmount = 200 * 10 ** 18; // initializing with 200 ETH as maximum

Please note that if the method reverts upon receiving zero safETH token, the impact still holds (i.e. Alice can not stake since the method will always revert).

Important: One could argue that this attack is expensive (although I don't beleive so), However, the attacker could always unstake and receive his funds back since the percentage of the asset is calculated based on the amount of safETH and the derivative's balance.

	uint256 derivativeAmount = (derivatives[i].balance() * _safEthAmount) / safEthTotalSupply;

That's why I set the severity to high because:

1. The attacker doesn't lose fund.
2. The attacker can keep the protocol DoSed as long as he/she wants.

Tools Used

Manual analysis

Recommended Mitigation Steps

In the derivative contract, don't rely on balanceOf(address(this)), instead keep track of the balance in a state variable. This way, manipulating the price (in the way decribed above) is not possible anymore.