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

Accounts with the `SIGNING_ROLE` can directly call `renounceRole` from AccessControl, causing state desynchronizt una participació equitativa de tots els signants en el flux de governança del multisig. * Només l’owner pot proposar transaccions, ja

Author Revealed upon completion

Accounts with the SIGNING_ROLE can directly call renounceRole from AccessControl, causing state desynchronization.

Description

Only the owner should be able to revoke the signer role, ensuring that any change in the multisig composition goes through the controlled flow that correctly updates all internal state.

Any signer can directly call renounceRole from AccessControl, removing their role without going through contract logic and without updating s_isSigner, s_signerCount, or the list of signers, causing critical state desynchronization.

AccessControl contract

@> function renounceRole(bytes32 role, address callerConfirmation) public virtual {
if (callerConfirmation != _msgSender()) {
revert AccessControlBadConfirmation();
}
​
_revokeRole(role, callerConfirmation);
}

Risk

Likelihood: High

  • Any signer can call renounceRole at any time, since the function is public and part of AccessControl.

  • The contract does not block or intercept this action, so the behavior is always accessible.

Impact: High

  • The internal state of the multisig becomes desynchronized, causing incorrect signer counts and possible permanent lockups.

  • “Ghost” signer slots may be occupied, preventing new signers from being added and breaking contract governance.

Proof of Concept

This test demonstrates that a signer can call renounceRole directly, losing the SIGNING_ROLE without the internal signer count being updated. This creates a desynchronization between the actual roles and the internal state of the multisig.

function test_SignerCanRevokeSignerRole() public {
// Define owner + 4 signers
address[5] memory signers;
signers[0] = OWNER;
signers[1] = SIGNER_TWO;
signers[2] = SIGNER_THREE;
signers[3] = SIGNER_FOUR;
signers[4] = SIGNER_FIVE;
​
// The owner assigns the signer role to all accounts
for (uint256 i = 1; i < signers.length; i++) {
vm.prank(OWNER);
multiSigTimelock.grantSigningRole(signers[i]);
}
​
bytes32 signerRole = keccak256("SIGNING_ROLE");
​
// Check that all have the signer role
for (uint256 i = 0; i < signers.length; i++) {
assertTrue(multiSigTimelock.hasRole(signerRole, signers[i]));
}
​
// Internal count indicates 5 signers
assertEq(multiSigTimelock.getSignerCount(), 5);
​
// A signer renounces their role directly via AccessControl
vm.prank(SIGNER_TWO);
multiSigTimelock.renounceRole(signerRole, SIGNER_TWO);
​
// Internal count does NOT update (desynchronization)
assertEq(multiSigTimelock.getSignerCount(), 5);
​
// The role is correctly removed
assertFalse(multiSigTimelock.hasRole(signerRole, SIGNER_TWO));
​
// The owner proposes a transaction
uint256 prop = multiSigTimelock.proposeTransaction(address(1), 10 ether, "");
​
// The signer who renounced can no longer confirm
vm.prank(SIGNER_TWO);
vm.expectRevert(
abi.encodeWithSelector(
IAccessControl.AccessControlUnauthorizedAccount.selector,
SIGNER_TWO,
bytes32(keccak256("SIGNING_ROLE"))
)
);
multiSigTimelock.confirmTransaction(prop);
}
[PASS] test_SignerCanRevokeSignerRole() (gas: 448067)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 32.96ms (5.90ms CPU time)
​

Recommended Mitigation

Override renounceRole to prevent signers from unilaterally renouncing the SIGNING_ROLE, forcing signer management to go exclusively through owner-controlled functions.
This solution preserves standard AccessControl behavior for other roles and prevents desynchronization of the multisig’s internal state.

​
+ error MultiSigTimelock__OnlyOwnerCanRevoke_SIGNING_ROLE();
​
+ function renounceRole(bytes32 role, address callerConfirmation) public override {
+ if (callerConfirmation != _msgSender()) {
+ revert AccessControlBadConfirmation();
+ }
​
+ if (role == SIGNING_ROLE) {
+ revert MultiSigTimelock__OnlyOwnerCanRevoke_SIGNING_ROLE();
+ }
​
+ super.renounceRole(role, callerConfirmation);
+ }

Support

FAQs

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

Give us feedback!