In Controller.reorderStrategies there's no check that _strategy1 and 2 are strategies of _vault. If the strategist by mistake calls this function using a different strategy, index evaluates as zero, effectively replacing the first vault's strategy with a non-appropriate one.
Consider adding require(_vaultStrategies[_strategy1] == _vault) (idem for _strategy2). These checks can also replace require(manager.allowedStrategies(_strategy1/2), "!_strategy1/2").
Handle
0xsanson
Vulnerability details
Impact
In
Controller.reorderStrategies
there's no check that_strategy1
and 2 are strategies of_vault
. If the strategist by mistake calls this function using a different strategy,index
evaluates as zero, effectively replacing the first vault's strategy with a non-appropriate one.Proof of Concept
https://github.com/code-423n4/2021-09-yaxis/blob/main/contracts/v3/controllers/Controller.sol#L215
Tools Used
editor
Recommended Mitigation Steps
Consider adding
require(_vaultStrategies[_strategy1] == _vault)
(idem for_strategy2
). These checks can also replacerequire(manager.allowedStrategies(_strategy1/2), "!_strategy1/2")
.