MultiSig Timelock

First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Submission Details
Impact: low
Likelihood: high
Invalid

L03. Discrepancy Between Documentation and Actual Role Assignment for Deployer

Root + Impact

Description

  • The documented behavior states that the deployer automatically receives both OpenZeppelin’s DEFAULT_ADMIN_ROLE and the custom SIGNING_ROLE, implying that AccessControl is used for administrative governance.

  • The actual implementation only grants the SIGNING_ROLE to the deployer and never assigns DEFAULT_ADMIN_ROLE, creating a discrepancy between documentation and code that can lead to incorrect assumptions about role administration and security guarantees.

constructor() Ownable(msg.sender) {
s_signers[0] = msg.sender;
s_isSigner[msg.sender] = true;
s_signerCount = 1;
// @> Only SIGNING_ROLE is granted
_grantRole(SIGNING_ROLE, msg.sender);
// @> DEFAULT_ADMIN_ROLE is never granted
}

Risk

Likelihood:

  • Documentation is referenced by integrators or auditors when assessing role governance and trust assumptions.

  • Future maintainers rely on documentation rather than code when extending or refactoring AccessControl logic.

Impact:

  • Incorrect threat modeling due to the assumption that AccessControl admin protections exist.

  • Potential misconfiguration or unsafe extensions that rely on a non-existent DEFAULT_ADMIN_ROLE.

Proof of Concept

Explanation

This test demonstrates that despite the documentation claim, the deployer does not possess the DEFAULT_ADMIN_ROLE. Any logic assuming AccessControl-based administration is therefore invalid.

function testDeployerDoesNotHaveDefaultAdminRole() public {
// Deployer is the test contract itself
address deployer = address(this);
// SIGNING_ROLE is correctly assigned
assertTrue(multiSigTimelock.hasRole(multiSigTimelock.getSigningRole(), deployer));
// DEFAULT_ADMIN_ROLE is NOT assigned
assertFalse(
multiSigTimelock.hasRole(
multiSigTimelock.DEFAULT_ADMIN_ROLE(),
deployer
)
);
}

Recommended Mitigation

Documentation and code must be aligned to avoid incorrect assumptions about governance.

Option 1: Update the code to match documentation (constructor)

constructor() Ownable(msg.sender) {
+ _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(SIGNING_ROLE, msg.sender);
}

Option 2: Update documentation to reflect actual behavior (preferred if AccessControl admin is unnecessary)

- Deployer receives DEFAULT_ADMIN_ROLE and SIGNING_ROLE
+ Deployer becomes owner and is granted SIGNING_ROLE only; AccessControl admin roles are not used

Either approach should be applied consistently to prevent governance misunderstandings.

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!