Broken access control leads to protocol functionality freeze

[code423n4]

Handle

0xRajeev

Vulnerability details

Impact

The contracts use an access control pattern where the contract deployer is included in the onlyDAO modifier which is used for authorized access to critical functions. Such contracts also include a purgeDeployer function which renounces (sets to zero-address) the deployer address and is expected to be called by deployer/DAO when the DAO is stable/final so that deployer no longer has access to critical functions.

However, not all such critical functions can be called by the DAO via proposals. This leads to functionality freeze for parameters/logic controlled by such functions once the deployer is purged because the DAO cannot call them anymore.

Such functions are: BondVault::release(), onlyDAO functions in Router, poolFactory::createPool and synthVault::setParams().

Impact: The protocol parameters set in above functions cannot be changed via the DAO. Zero liquidity pools cannot be created via the DAO.

Proof of Concept

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/BondVault.sol#L141-L143

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Router.sol#L332-L345

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/poolFactory.sol#L64-L77

https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/synthVault.sol#L81-L85

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add DAO functions to allow the above functions to be called via proposals. If these are not intended to be set by DAO and only by deployer before being purged, change the modifier and document appropriately.

[SamusElderg]

Duplicate of #172