Submitted by pfapostol
The removeStrategy function in the MultiStrategy contract allows the removal of a strategy and redistributes the withdrawn funds among the remaining strategies.
The issue arises when the last strategy is removed. The weight (_weights[index]) of the last strategy is first subtracted from _totalWeight, which results in _totalWeight being zero, and it is then set to zero.
Later, when _allocateAssets is called: for each of the active _strategies (the last strategy has not yet been removed), it attempts to calculate the fraction of the input amount. However, since _totalWeight is zero, the execution is reverted with a panic: division or modulo by zero error.
The VAULT_MANAGER_ROLE would be unable to delete the last strategy.
This PoC follows these steps:
However, the execution fails with:
There are several ways to improve the code to fix the issue (such as adding a check for zero, etc.). However, the most straightforward and direct approach is to remove the strategy from _strategies before calling _allocateAssets:
chefkenji (BakerFi) confirmed
0xpiken (warden) commented:
pfapostol (warden) commented:
Dravee (judge) commented:
BakerFi mitigated:
Status: Mitigation confirmed. Full details in reports from shaflow2 and 0xlemon.
