alcueca
commented
3 years ago These contracts allow for separation of concerns, but they can't and don't intend to enforce it. If a check would be implemented there would be multiple ways of bypassing it, such as granting roles (which is allowed, we can have multiple planners and/or executors).
ghoul-sol
Handle
gpersoon
Vulnerability details
Impact
The contracts TimeLock and EmergencyBrake want to enforce separation of concerns. However there is no check that scheduler/planner is different from executor.
If they would be the same there is no separation of concerns.
Proof of Concept
//https://github.com/code-423n4/2021-08-yield/blob/main/contracts/utils/TimeLock.sol#L29 constructor(address scheduler, address executor) AccessControl() {
//https://github.com/code-423n4/2021-08-yield/blob/main/contracts/utils/EmergencyBrake.sol#L34 constructor(address planner, address executor) AccessControl() {
Tools Used
Recommended Mitigation Steps
Add something like the following in the constructor of TimeLock: require (scheduler!=executor,"The same");
Add something like the following in the constructor of EmergencyBrake: require (planner!=executor,"The same");