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

0 stars 0 forks source link

Missing timelock for critical contract setters of privileged roles #37

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Setter functions for critical contract parameters accessible only by privileged roles e.g. onlyGovernance or onlyStrategist should consider adding timelocks (along with events) so that users and other privileged roles can detect upcoming changes and have the time to react to them.

Changes to fees, dividends, contract addresses etc. may have a financial or trust impact on users who should be given an opportunity to react to them by exiting/engaging without being surprised when such changes are made effective immediately.

None of the setters in the protocol have a timelock controller functionality to delay enforcement of the critical changes they make.

Proof of Concept

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Vault.sol#L69-L77

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Vault.sol#L84-L93

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Vault.sol#L101-L109

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

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/LegacyController.sol#L53-L64

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/LegacyController.sol#L70-L77

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/controllers/LegacyController.sol#L83-L90

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/strategies/BaseStrategy.sol#L95-L104

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L242-L251

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L258-L267

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L273-L281

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L288-L297

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L303-L311

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L318-L327

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L351-L360

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L367-L376

https://github.com/code-423n4/2021-09-yaxis/blob/cf7d9448e70b5c1163a1773adb4709d9d6ad6c99/contracts/v3/Manager.sol#L383-L392

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider adding timelocks to such contracts with critical setter functions.

GalloDaSballo commented 3 years ago

Agree that some of these setters should be behind timelock However, this is out of scope for the contest