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

OpenZeppelin AccessControl `renounceRole()` function not implemented, causing ghost signers.

Author Revealed upon completion

Root + Impact

Description

OpenZeppelin's AccessControl allows any role holder to call renounceRole(bytes32 role, address account) where account must be msg.sender. When a signer calls this function, they lose the SIGNING_ROLE but remain in the s_signers[] array with s_isSigner[signer] = true and are still counted in s_signerCount. This creates "gost signers" who:

  • Cannot confirm transactions (no role)

  • Still count toward MAX_SIGNER_COUNT

  • Block addition of new functional signers

  • Are not removed from s_signers[] array

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

Risk

Likelihood:

  • The renounceRole() function is a standard function from the inherited library, therefore it could be called intentionally or unintentionally.

Impact:

  • Only the OpenZeppelin implementation of a role is renounced, leaving all state variables unchanged; the signer itself remains active but useless.

  • New signers cannot be added when compromised signers exist.

  • Reduces effective signer count (5→4→3).

  • At 3 signers, becomes 3-of-3 (no redundancy, single point of failure).

  • Ghost signers create confusion and management overhead.

  • Inconsistent state between AccessControl and multisig tracking

Proof of Concept

Add this snipped of code to the MultiSigTimelockTest.t.sol test file.

function test_signerCallsRenounceRoleGostSignerRemains() public grantSigningRoles {
// ARRANGE
// ACT
vm.startPrank(SIGNER_TWO);
multiSigTimelock.renounceRole(multiSigTimelock.getSigningRole(), SIGNER_TWO);
vm.stopPrank();
​
// ASSERT
// after renounce signers still counted
assertEq(multiSigTimelock.getSignerCount(), 5, "Should still be 5 signers");
assertFalse(multiSigTimelock.hasRole(multiSigTimelock.getSigningRole(), SIGNER_TWO));
}

How to execute:

forge test --mt test_signerCallsRenounceRoleGhostSignerRemains -vvvv

Recommended Mitigation

Override `renounceRole` to properly delete signer from the protocol.

+ function renounceRole(bytes32 role, address account) public virtual override {
+ require(account == msg.sender, "Can only renounce own roles");
+
+ if (role == SIGNING_ROLE) {
+ // Integrate with multisig state
+ if (s_signerCount <= 1) {
+ revert MultiSigTimelock__CannotRevokeLastSigner();
+ }
+
+ // Find and remove from array (same logic as revokeSigningRole)
+ uint256 indexToRemove = type(uint256).max;
+ for (uint256 i = 0; i < s_signerCount; i++) {
+ if (s_signers[i] == account) {
+ indexToRemove = i;
+ break;
+ }
+ }
+
+ if (indexToRemove < s_signerCount - 1) {
+ s_signers[indexToRemove] = s_signers[s_signerCount - 1];
+ }
+ s_signers[s_signerCount - 1] = address(0);
+ s_signerCount -= 1;
+ s_isSigner[account] = false;
+ }
+
+ super.renounceRole(role, account);
+ }

Support

FAQs

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

Give us feedback!