Incorrect access control on Harvester add/remove strategy functions

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

The documentation (comments) indicate that addStrategy() and removeStrategy() are gov/strategist only functions which is true for setHarvester() and setSlippage() but add/remove strategy have the incorrect onlyController modifier instead of onlyStrategist.

Proof of Concept

Comment: https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Harvester.sol#L90-L92

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Harvester.sol#L100-L107

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Harvester.sol#L120-L127

Tools Used

Manual Analysis

Recommended Mitigation Steps

If this is the correct authorization, change the comment/documentation. If not, change the modifier to onlyStrategist.

[Haz077]

addStrategy() and removeStrategy() are meant to have onlyController modifier.

[GalloDaSballo]

Agree with sponsor, controller is going to be called by governance and will then enact the change