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

0 stars 0 forks source link

Halting the protocol should be onlyGovernance and not onlyStrategist #47

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

A malicious strategist can halt the entire protocol and force a permanent shutdown once they observe that governance is trying to set a new strategist and they do not agree with that decision. They may use the 7 day window to halt the protocol. The access control on setHalted() should be onlyGovernance.

Proof of Concept

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

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

Tools Used

Manual Analysis

Recommended Mitigation Steps

Change access control to onlyGovernance from onlyStrategist for setHalted()

uN2RVw5q commented 3 years ago

Implemented in https://github.com/code-423n4/2021-09-yaxis/pull/35

GalloDaSballo commented 3 years ago

Agree that such critical functionality should be limited to the highest permission access role. Sponsor has mitigated