search

code-423n4 / 2023-05-asymmetry-mitigation-findings

2 stars 2 forks source link

[H-03] mitigation error: Reenabling a derivative forces a sudden uncontrollable weight redistribution #74

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

[H-03] mitigation error: Reenabling a derivative forces a sudden uncontrollable weight redistribution

Mitigated issue

H-03: Users can fail to unstake and lose their deserved ETH because malfunctioning or untrusted derivative cannot be removed.

New issue within mitigation: Reenabling a derivative forces a sudden uncontrollable weight redistribution

The revised adjustWeight() now prevents weights of disabled derivatives from being changed, by

require(settings[_derivativeIndex].enabled, "derivative not enabled");

But enableDerivative() and disableDerivative() do not change the derivative's weight setting, but only recalculates the total weight of enabled derivatives. Therefore, reenabling a derivative forces a sudden uncontrollable weight redistribution. If one wishes to reenable a derivative with a new weight setting, this can only be done by a second call to adjustWeight() after it has been enabled. This is different from when adding a derivative, where the derivative and its weight are specified simultaneously. Thus, while disabling a derivative is very much like removing it, enabling a derivative is not like adding it.

The issue is therefore that the newly reenabled derivative may be called its desired new weight has been set.

For example, let's say a temporarily malfunctioning derivative has been disabled. Later the issue with the derivative is fixed and the owner wishes to bring it back to the protocol. But, either because he wants to do so cautiously or because circumstances have changed since it was disabled, the weight it had when disabled is no longer appropriate. Then the owner cannot reenable the derivative with its desired weight, which might lead to users staking with unintended weights.

Recommendation

Since a disabled derivative is always skipped I see no reason why the weight of a disabled derivative should not be adjustable. It has no effect while the derivative is disabled. But allowing adjustment of the weight while it is disabled, i.e. before it is enabled, resolves this new issue.

toshiSat commented 1 year ago

Valid, but fine with this functionality

liveactionllama commented 1 year ago

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.