Mitigation Confirmed for Mitigation of M-10: Issue mitigated

[code423n4]

Mitigated issue

M-10: Stuck ether when use function stake with empty derivatives(derivativeCount = 0)

The issue was that stake() will accept payment but not issue safETH when derivativeCount == 0 or when all weights[i] == 0.

Mitigation review

The proposed mitigation simply adds a require(derivativeCount > 0, "no derivatives");. This solves only the issue when derivativeCount == 0.

The latest branch does not have this but has instead require(totalWeight > 0, "total weight is zero");. This solves the issue when all weights[i] == 0. Since a weight can be set only on added derivatives, derivativeCount > 0 only if totalWeight > 0, so this also solves the first issue.

However, we now also have this line:

+ if (!derivatives[i].enabled) continue;
uint256 weight = derivatives[i].weight;
if (weight == 0) continue;

Therefore we could have a problem if the derivative is skipped because its disabled, but has non-zero weight. But since totalWeight only counts enabled derivatives and is updated whenever weights or enabled/disabled status change, if all derivatives are skipped, for either reason, this would imply that totalWeight == 0, so it is sufficient to check that totalWeight > 0.

Comment on rebalanceToWeights()

Note that we divide by totalWeight inside a for-loop in stake(). So if totalWeight == 0 this would naturally revert. The problem was that the loop was skipped if derivativeCount == 0, so that the sent funds fail to deposit. We fixed this by adding that check before the loop.

The same logic has a parallel in rebalanceToWeights(), but in this case nothing is withdrawn if derivativeCount == 0. Therefore the revert by dividing by zero prevents the same issue from appearing in rebalanceToWeights(). But it may be wise to refactor to make this check explicit, like in stake().

[elmutt]

thanks

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as duplicate of #54