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

19Dec2025_AuditReport1_MultiSigTimelock

Author Revealed upon completion

Root + Impact

Description

  • Inconsistent State Between s_isSigner Mapping and AccessControl Roles

  • The contract maintains two separate sources of truth for signer status: the s_isSigner mapping and the AccessControl SIGNING_ROLE. While these are updated together in normal operations, there's no guarantee they remain synchronized. The contract uses s_isSigner for validation in revokeSigningRole but SIGNING_ROLE for actual permission checks. If these become desynchronized (e.g., through direct AccessControl manipulation if the contract is upgraded or through inheritance issues), it could lead to authorization bypasses

// Root cause in the codebase with @> marks to highlight the relevant section
function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// CHECKS
if (!s_isSigner[_account]) { // Checks s_isSigner
revert MultiSigTimelock__AccountIsNotASigner();
}
// ...
s_isSigner[_account] = false;
_revokeRole(SIGNING_ROLE, _account); // Updates AccessControl
}
​
function confirmTransaction(uint256 txnId)
external
nonReentrant
transactionExists(txnId)
notExecuted(txnId)
onlyRole(SIGNING_ROLE) // Uses AccessControl for auth
{

Risk

Likelihood:

  • When the contract were to add functionality that directly calls AccessControl's grantRole without updating s_isSigner, or if there's an upgrade path that doesn't properly migrate both states, an attacker could gain signing privileges without being tracked in the signer array.

Impact:

  • Users could potentially confirm/execute transactions without being in the s_signers array, or legitimate signers could be prevented from acting if the two systems desynchronize.

Proof of Concept

function revokeSigningRole(address _account) external nonReentrant onlyOwner noneZeroAddress(_account) {
// CHECKS
if (!s_isSigner[_account]) { // Checks s_isSigner
revert MultiSigTimelock__AccountIsNotASigner();
}
// ...
s_isSigner[_account] = false;
_revokeRole(SIGNING_ROLE, _account); // Updates AccessControl
}
​
function confirmTransaction(uint256 txnId)
external
nonReentrant
transactionExists(txnId)
notExecuted(txnId)
onlyRole(SIGNING_ROLE) // Uses AccessControl for auth
{

Recommended Mitigation

// Use a single source of truth for signer status. Either rely entirely on AccessControl or entirely on the custom mapping:
// Option 1: Remove s_isSigner and use only AccessControl
function revokeSigningRole(address _account) external nonReentrant onlyOwner {
if (!hasRole(SIGNING_ROLE, _account)) {
revert MultiSigTimelock__AccountIsNotASigner();
}
// Find in array using hasRole instead of s_isSigner
// ...
}
​
// Option 2: Add a consistency check
modifier consistentSignerState(address account) {
require(s_isSigner[account] == hasRole(SIGNING_ROLE, account), "Inconsistent state");
_;
}

Support

FAQs

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

Give us feedback!