Inaccurate `ethPerDerivative()` may cause a loss for holders of safETH when derivative distribution in unstable

[code423n4]

Inaccurate ethPerDerivative() may cause a loss for holders of safETH when derivative distribution in unstable

https://github.com/asymmetryfinance/smart-contracts/blob/ec582149ae9733eed6b11089cd92ca72ee5425d6/contracts/SafEth/SafEth.sol#L87-L119

Impact

When the derivative balances are not distributed according to their weights (for example after a weight change or addition of a derivative), staking may mint too many (or too few) safETH, which represents a loss (or gain) for other holders of safETH.

Proof of Concept

Since changes in the markets naturally affect the value of safETH, in order to speak of the vault intrinsic value of safETH (to determine the amount to mint) we assume that the market is perfectly stable. We further assume that the price of derivative tokens don't change due to safETH stakes and unstakes (e.g. from changing the balance in the liquidity pools).

What we want for fair staking and unstaking is that the return values of stake() and unstake() are independent of total supply of safETH. In other words, staking and unstaking shouldn't by itself change the value of safETH. Otherwise a staker might for example cause a loss to already existing holders of safETH.

The amount to receive when unstaking is encoded in the amount of safETH that is unstaked. This is the definition of a share of safETH. Therefore unstake() is by definition fair.

Therefore the correct amount to mint must be determined when staking. Unfortunately this presents the following problem.

Let $p_i$ and $b_i$ be the price in ETH and the balance of derivative $i$, and let $d_i$ be the amount deposited into derivative $i$ (the weights are factored into this). The amount to mint is then $\frac{\Sigma p_i d_i}{\Sigma p_i b_i} \Sigma b_i$. Let's single out derivative $k$ and denote the sum of the remaining values of deposits $D$ and the sum of the remaining values of balances $B$, and we have $\frac{D + p_k d_k}{B + p_k b_k} \Sigma b_i$. The derivative of this with respect to $p_k$ is $\frac {Bd_k - Db_k}{(b_k p_k + B)^2}\Sigma b_i$. So when $Bd_k - Db_k > 0$ the derivative is positive. That is, when $\frac{B}{b_k} > \frac{D}{d_k}$ an increase in $p_k$ causes more safETH to be minted. Now, if the distribution of derivatives is the same as the weights then staking ETH will cause an equal proportion to be deposited in derivative $k$ as in all other derivatives, and hence also in the sum of the other derivatives, i.e. $\frac{B}{b_k} = \frac{D}{d_k}$. So when the distribution is stable it doesn't matter if $p_k$ is inaccurate.

All of this means that if the price per derivative is inaccurate and the weights have been changed (or derivative added) within recent time, an invalid amount of safETH is minted, and unfortunately the amount may vary in the same as well as the opposite direction of the inaccuracy of the price.

[toshiSat]

This is why we have implemented slippage protection on both the derivative and input level. This is valid and unfortunately due to the state of oracle prices on Ethereum

[liveactionllama]

Per discussion with the judge, marking this as QA. Also, see their comment here for more detail.

Since QA are not eligible for awards, nullifying.