Mitigation Confirmed for Mitigation of H-03: Issue mitigated with error

code423n4 commented 1 year ago

Mitigated issue

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

The issue was that unstake() and rebalanceToWeights() always call every derivative's withdraw(), but that withdraw() may potentially revert for a particular malfunctioning derivative, DoS-ing unstake() and rebalanceToWeights().

Mitigation review

weights has been replaced by settings which consists of both the derivative's weight and its enabled status. Whenever a derivative is about to be called, if it is not enabled it is simply skipped. Functions to enable/disable derivatives have been added (enableDerivative() and disableDerivative()). All changes to weights or enabled status correctly recalculates the total weight only for enabled derivatives (by the newly added setTotalWeight()). This eliminates this issue.

There is in principal still a possibility of an out-of-gas revert when looping over the derivative list, but it seems entirely unrealistic that the list would ever grow this long.

Additional comments

The error messages in

function disableDerivative(uint256 _derivativeIndex) external onlyOwner {
    ...
    require(settings[_derivativeIndex].enabled, "derivative not enabled");
    ...
}

function enableDerivative(uint256 _derivativeIndex) external onlyOwner {
    ...
    require(
        !settings[_derivativeIndex].enabled,
        "derivative already enabled"
    );
    ...
}

seem inconsistent. The error message in disableDerivative() should probably also be "derivative already disabled".

Mitigation error

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

elmutt commented 1 year ago

thanks

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

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