MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

Missing `DEFAULT_ADMIN_ROLE` grant in constructor breaks `grantRole()` and `revokeRole()` functionality

yallthedev

Missing DEFAULT_ADMIN_ROLE grant in constructor breaks grantRole() and revokeRole() functionality

Description

OpenZeppelin's AccessControl requires a caller to have the admin role of the target role to grant or revoke it. By default, SIGNING_ROLE's admin is DEFAULT_ADMIN_ROLE (bytes32(0)).

In the constructor, the owner is only granted SIGNING_ROLE but not DEFAULT_ADMIN_ROLE, making the inherited grantRole() and revokeRole() functions unusable.

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: High

  • This occurs every time the contract is deployed, as the constructor never grants DEFAULT_ADMIN_ROLE

Impact: Medium

  • The grantRole() and revokeRole() functions inherited from OpenZeppelin's AccessControl are completely broken

  • While custom grantSigningRole() and revokeSigningRole() functions work, the standard AccessControl interface is non-functional

Proof of Concept

Explanation: The owner attempts to call grantRole() to add a new signer. Despite being the contract owner and having SIGNING_ROLE, the call reverts with AccessControlUnauthorizedAccount because the owner lacks DEFAULT_ADMIN_ROLE which is required to manage SIGNING_ROLE.

function testFuzz_GrantRole_AdminCanGrant(address account) public {
bytes32 signingRole = multiSigTimelock.getSigningRole();
// Owner tries to grant SIGNING_ROLE via inherited grantRole()
// Reverts: AccessControlUnauthorizedAccount(owner, DEFAULT_ADMIN_ROLE)
multiSigTimelock.grantRole(signingRole, account);
}

Expected: Owner can use grantRole() to manage signer roles.
Actual: Transaction reverts with AccessControlUnauthorizedAccount.

Recommended Mitigation

Explanation: Grant DEFAULT_ADMIN_ROLE to the deployer in the constructor. This allows the owner to use both the custom functions (grantSigningRole/revokeSigningRole) and the standard AccessControl interface (grantRole/revokeRole).

constructor() Ownable(msg.sender) {
s_signers[0] = msg.sender;
s_isSigner[msg.sender] = true;
s_signerCount = 1;
​
+ _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(SIGNING_ROLE, msg.sender);
}
Updates

Lead Judging Commences

kelechikizito Lead Judge
10 days ago
kelechikizito Lead Judge 4 days ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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

Give us feedback!