Asymmetry contest - Stiglitz's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L73 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L91 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L115 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L118 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L141 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L142 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L152

Vulnerability details

Description

New derivates can be added but not removed in the contract SafeEth. It can lead to the DoS, when the code of the derivate is reverting after the upgrade. (Link to the affected code points to the function calls that can cause the risk. However, it is not the code that is necessary to change)

Vulnerable scenarion

A new derivate XXX is added by the function addDerivate. Derivate must implement the interface IDerivate. The derivate XXX can be an upgradeable contract as contracts in the repository are (Reth, SfrxEth and WstEth). Functions of all added contracts are called inside the functions stake and unstake. These functions are defined by the mentioned interface IDerivate. For example, in stake function at line #73 the:

derivatives[i].ethPerDerivative(derivatives[i].balance()

In the unstake function at line #118:

derivatives[i].withdraw(derivativeAmount);

When the code of the derivate XXX is upgraded and one of these functions is upgraded incorrectly by accident or maliciously (stolen ownership of XXX contract) and reverts, it will DoS the whole contract. Depending on the reverting function, it will be possible to stake/unstake. The only way to mitigate DoS impact after this happens is to upgrade the SafeEth contract.

Recommended Mitigation Steps

Setting the weight of the "broken" derivate to zero is not enough. Adding the functionality to delete the derivate seems reasonable. However, it will be necessary to consider the logic of how to delete the derivate and not harm the users. For example, manually rebalance the weights.