MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: medium

OpenZeppelin AccessControl admin role initialization issue: owner not granted `DEFAULT_ADMIN_ROLE`

Author Revealed upon completion

Root + Impact

Description

The constructor grants the SIGNING_ROLE to the deployer but does not grant the DEFAULT_ADMIN_ROLE. This prevents the use of standard OpenZeppelin AccessControl functions, such as direct grantRole() and revokeRole() calls, creating inconsistency with expected patterns and violating the protocol's intended behaviour according to the documentation in README.md, which states:

  • Automatically receives both the OpenZeppelin DEFAULT_ADMIN_ROLE and the custom SIGNING_ROLE, becoming the first signer.

constructor() Ownable(msg.sender) {
// Automatically add deployer as first signer
s_signers[0] = msg.sender;
s_isSigner[msg.sender] = true;
s_signerCount = 1;
​
// Grant signing role to deployer
_grantRole(SIGNING_ROLE, msg.sender);
@> // Missing: _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
}

Risk

Likelihood:

  • Missed in the constructor, this has an effect throughout the contract's life until such a mistake is uncovered.

Impact:

  • Cannot use grantRole(SIGNING_ROLE, account) directly

  • Cannot use revokeRole(SIGNING_ROLE, account) directly

  • Forces use of custom grantSigningRole / revokeSigningRole

  • Inconsistent with OpenZeppelin patterns

  • Low severity since custom functions exist

Proof of Concept

Add this snipped of code to the MultiSigTimelockTest.t.sol test file.

function test_ownerIsDefaultAdmin() public deployScript {
assertTrue(multiSigTimelock.hasRole(multiSigTimelock.DEFAULT_ADMIN_ROLE(), OWNER));
}

How to execute:

forge test --mt test_ownerIsDefaultAdmin -vvvv

Recommended Mitigation

Add _grantRole(DEFAULT_ADMIN_ROLE, msg.sender) to the contract constructor.

constructor() Ownable(msg.sender) {
// Automatically add deployer as first signer
s_signers[0] = msg.sender;
s_isSigner[msg.sender] = true;
s_signerCount = 1;
​
// Grant signing role to deployer
_grantRole(SIGNING_ROLE, msg.sender);
+ _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!