[WP-H1] Timelock can be hijacked by a malicious/compromised deployer

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-rolla/blob/efe4a3c1af8d77c5dfb5ba110c3507e67a061bdd/quant-protocol/contracts/timelock/TimelockController.sol#L78-L80

Vulnerability details

The purpose of a ConfigTimelockController contract is to put a limit on the privileges of the admin, by forcing a two step process by two different roles with a preset delay time.

However, we found that the current implementation actually won't serve that purpose as it allows the admin to execute transactions without any constraints under certain circumstances.

If the initial admin (deployer) is malicious or compromised, the attacker can call grantRole() grant TIMELOCK_ADMIN_ROLE to an address controlled by the attacker, and the revoke the TIMELOCK_ADMIN_ROLE of the the initial admin (deployer) and all other roles held by other addresses.

Then they can grant PROPOSER_ROLE and EXECUTOR_ROLE to themselves and do pretty much anything afterwards.

In conclusion, a Timelock contract is supposed to guard the protocol from lost private key or malicious actions. The current implementation makes it possible for the admin not to obey the desired constraints, and they can hijack the whole timelock.

https://github.com/code-423n4/2022-03-rolla/blob/efe4a3c1af8d77c5dfb5ba110c3507e67a061bdd/quant-protocol/contracts/timelock/TimelockController.sol#L78-L80

// deployer + self administration
_setupRole(TIMELOCK_ADMIN_ROLE, _msgSender());
_setupRole(TIMELOCK_ADMIN_ROLE, address(this));

PoC

A malicious/compromised deployer can:

1. Grant TIMELOCK_ADMIN_ROLE to an address controlled by the attacker;
2. Revoke TIMELOCK_ADMIN_ROLE role of the current deployer's address;
3. Revoke roles of all other addresses;
4. Schedule and execute whatever transactions.

Recommendation

Consider removing _setupRole(TIMELOCK_ADMIN_ROLE, _msgSender()); at L79.

[quantizations]

As part of the contest guidelines, it was to be assumed this was the deployment configuration: https://docs.rolla.finance/rolla/quant-protocol/deployments

The role is assigned in the code above but does not remain in the documented configuration as it is renounced in the deployment scripts

[alcueca]

Dispute accepted.