search

code-423n4 / 2021-07-spartan-findings

0 stars 0 forks source link

Unnecessary/Incorrect onlyDAO modifier could be an indication of missing access control #140

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Unnecessary/incorrect access control modifier is typically an indication of missing critical authorization checks. The onlyDAO modifier (used in various protocol contracts) is present in synthFactory.sol but used only in the purgeDeployer (which sets deployer to 0 address) and is also incorrect in that it only checks against DEPLOYER and not the DAO address. This is effectively unnecessary.

Proof of Concept

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/synthFactory.sol#L21-L25

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/synthFactory.sol#L31-L34

Tools Used

Manual Analysis

Recommended Mitigation Steps

Re-evaluate access control logic in modifier and purgeDeployer() to either use this modifier for other functions or else remove it along with purgeDeployer().

SamusElderg commented 3 years ago

Duplicate of #172

ghoul-sol commented 3 years ago

Duplicate of #172