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

Owner Can Grant Signing Role Without Holding Admin Role (Access Control Bypass)

Author Revealed upon completion

Root + Impact

The contract combines OpenZeppelin’s Ownable and AccessControl modules but incorrectly restricts sensitive signer-management functions (grantSigningRole and revokeSigningRole) using onlyOwner instead of role-based authorization (onlyRole(DEFAULT_ADMIN_ROLE)).
Additionally, signer roles are assigned using _grantRole, which bypasses AccessControl’s admin checks.

As a result, any address that becomes the contract owner—via a legitimate transferOwnership—can grant or revoke the SIGNING_ROLE without holding the administrative role, effectively bypassing the intended role-based access control system and enabling privilege escalation.

Description

  • In a properly designed role-based multi-signature wallet, adding or removing signers—who collectively control fund execution—must be restricted to an explicit administrative role (DEFAULT_ADMIN_ROLE).

  • Although this contract uses OpenZeppelin’s AccessControl, signer management is protected only by onlyOwner, and role assignment is performed via _grantRole, which does not enforce admin authorization. Ownership can be transferred freely using transferOwnership, after which the new owner gains the ability to add or remove signers despite not holding the administrative role.

  • This creates an access control bypass where ownership implicitly grants signer-management privileges, undermining the intended role hierarchy.

contract MultiSigTimelock is Ownable, AccessControl, ReentrancyGuard {}
​
​
constructor() Ownable(msg.sender) {
// ...
_grantRole(SIGNING_ROLE, msg.sender); // @> Only SIGNING_ROLE granted, DEFAULT_ADMIN_ROLE never assigned
}
​
​
function grantSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// ...
_grantRole(SIGNING_ROLE, _account); // @> Protected only by onlyOwner, not by admin role
}
​
​
function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// ...
_revokeRole(SIGNING_ROLE, _account); // @> AccessControl bypass: Only owner restriction allows signer removal without admin privileges
}
​

Risk

Likelihood:

  • Ownership transfer is a common and intended operation in Ownable contracts when teams change administrators or migrate control.

  • Many projects transfer ownership to a new address or multisig without realizing it also transfers signer management rights.

Impact:

  • A new owner (or compromised former owner who transferred ownership) can add malicious signers, achieving quorum and draining funds after the required timelock.

  • The role-based security model advertised in the documentation is undermined, reducing trust in the wallet’s governance model.

Proof of Concept

// 2025-12-multisig-timelock/test/unit/MultiSigTimelockTest.t.sol
​
function test_OwnerWithoutAdminCanGrantSignerRole() public {
// attacker address
address attacker = makeAddr("attacker");
​
// OWNER transfers ownership to attacker
multiSigTimelock.transferOwnership(attacker);
​
// sanity: attacker is NOT admin
bool isAdmin = multiSigTimelock.hasRole(
multiSigTimelock.DEFAULT_ADMIN_ROLE(),
attacker
);
assertFalse(isAdmin);
​
// attacker can still grant signing role
vm.prank(attacker);
multiSigTimelock.grantSigningRole(attacker);
​
// attacker is now signer
assertTrue(
multiSigTimelock.hasRole(
multiSigTimelock.getSigningRole(),
attacker
)
);
​
assertEq(multiSigTimelock.getSignerCount(), 2);
}

Recommended Mitigation

- constructor() Ownable(msg.sender) {
+ constructor() {
// ...
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
_grantRole(SIGNING_ROLE, msg.sender);
}
​
- function grantSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
+ function grantSigningRole(address _account) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) noneZeroAddress(_account) {
​
- function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
+ function revokeSigningRole(address _account) external nonReentrant onlyRole(DEFAULT_ADMIN_ROLE) noneZeroAddress(_account) {

Support

FAQs

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

Give us feedback!