`BaseVaultAdaptor.strategiesLength` must be updated manually

[code423n4]

Handle

cmichel

Vulnerability details

Vulnerability Details

The BaseVaultAdaptor has a storage variable that counts the number of strategies called strategiesLength. It must be manually set by setStrategiesLength.

For example, it's used in invest:

uint256[] memory targetRatios = _controller().getStrategiesTargetRatio();
for (uint256 i; i < strategiesLength; i++) {
    // accesses targetRatios[i]
    if (currentRatios[i] < targetRatios[i] && ...) {
        update = true;
        break;
    }
}

This function accesses targetRatios[i] for all i=0..strategiesLength, but targetRatios = controller.getStrategiesTargetRatio() = IInsurance(insurance).getStrategiesTargetRatio(utilRatio) = allocation.calcStrategyPercent(utilRatio) is an array of length 2. Any strategiesLength value greater than two will make it impossible to invest as the function reverts with an out of bounds exception.

Impact

It's often used as an iteration bound and therefore setting the wrong value can miss or break strategies.

Recommended Mitigation Steps

Check if it's possible to automatically determine the number of strategies instead of having to set this value manually. Check that the arrays that are accessed are all actually of length strategiesLength.

[kitty-the-kat]

17

[ghoul-sol]

Duplicate of #17 so a non-critical issue. With proper setup it works even is it's cumbersome.