search

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

0 stars 0 forks source link

Missleading onlyDAO modifiers #172

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Several contracts implement an onlyDAO modifier which, as the name suggests, should only authorize the function to be executed by the DAO. However, some implementations are wrong and either allow the DAO or the deployer to execute, or even only the deployer:

Incorrect implementations:

Impact

In all of these functions, the deployer may execute the function as well which is a centralization risk. The deployer can only sometimes be purged, as in synthFactory, in which case nobody can execute these functions anymore.

Recommended Mitigation Steps

Rename it to onlyDeployer or onlyDeployerOrDAO depending on who has access.

verifyfirst commented 3 years ago

This is by design a choice. However, there are current discussions around renaming the high level access modifiers to be more descriptive in their purpose.

ghoul-sol commented 3 years ago

This is a non-critical issue because there's no in-code bugs, it's rather error-prone naming.

ghoul-sol commented 3 years ago

On second look, I'll keep it a medium risk as deployer cannot be purged in all contracts which introduces systemic risk.