code423n4 commented 2 years ago

Handle

0xsanson

Vulnerability details

Impact

In Controller.sol it's possible to change maxStrategies to a different value. There's no check however that the new value is greater than the present number of strategies.

Proof of Concept

https://github.com/code-423n4/2021-09-yaxis/blob/main/contracts/v3/controllers/Controller.sol#L316

Tools Used

editor

Recommended Mitigation Steps

It's possible that this is an intended behaviour. If not, add a check for _maxStrategies >= _vaultDetails[_vault].strategies.length.

Haz077 commented 2 years ago

In my opinion, that should be taken care of when resetting maxStrategies, also it will not cause any bug in removing/adding other strategies.

GalloDaSballo commented 2 years ago

maxStrategies ends up being usable exclusively to gatekeep further strategies from being added

That said the finding from the warden is correct, the variable could be set to a number lower than the current number of strategies

This has no impact on the system, beside offering incorrect information

I believe Low Risk is correct here

code-423n4 / 2021-09-yaxis-findings