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

Missing DEFAULT_ADMIN_ROLE Grant in Constructor - AccessControl Broken

Author Revealed upon completion

Description

  • According to the protocol specification: "The account that deploys the MultiSigTimelock contract automatically receives both the OpenZeppelin DEFAULT_ADMIN_ROLE and the custom SIGNING_ROLE"

  • However, the constructor only grants SIGNING_ROLE to the deployer, not DEFAULT_ADMIN_ROLE. This means the AccessControl role management inherited from OpenZeppelin is completely non-functional.

constructor() Ownable(msg.sender) {
// Automatically add deployer as first signer
s_signers[0] = msg.sender;
s_isSigner[msg.sender] = true;
s_signerCount = 1;
​
// @> Only SIGNING_ROLE is granted, DEFAULT_ADMIN_ROLE is missing!
_grantRole(SIGNING_ROLE, msg.sender);
// @> Missing: _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
}

Risk

Likelihood:

  • This is a deployment-time bug that affects every contract deployment

  • 100% of deployments will have broken AccessControl

Impact:

  • The inherited AccessControl grantRole() and revokeRole() functions cannot be used by anyone

  • Protocol documentation claims DEFAULT_ADMIN_ROLE is granted but it's not

  • If the owner (Ownable) is compromised or lost, there's no backup admin mechanism through AccessControl

  • Inconsistency between documented behavior and actual implementation

Proof of Concept

function testDefaultAdminRoleNotGranted() public view {
bytes32 defaultAdminRole = multiSigTimelock.DEFAULT_ADMIN_ROLE();
// Owner should have DEFAULT_ADMIN_ROLE according to docs, but doesn't
assertFalse(multiSigTimelock.hasRole(defaultAdminRole, OWNER));
// This means no one can use AccessControl's grantRole/revokeRole
// The inherited functions are dead code
}
​
function testAccessControlGrantRoleFails() public {
bytes32 signingRole = multiSigTimelock.getSigningRole();
address newSigner = makeAddr("newSigner");
// Even owner cannot use inherited grantRole because they don't have DEFAULT_ADMIN_ROLE
vm.prank(OWNER);
vm.expectRevert(); // AccessControl: account is missing role
multiSigTimelock.grantRole(signingRole, newSigner);
}

Recommended Mitigation

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

Support

FAQs

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

Give us feedback!