MultiSig Timelock

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

renounceOwnership() function can brick the wallet

Author Revealed upon completion

Description

  • In a governed multisig wallet, the owner (admin) is expected to remain in control of administrative actions (e.g., adding/removing signers, proposing transactions if owner‑gated) or follow a safe two‑step ownership transfer. Renouncing ownership should be either disabled or guarded to avoid leaving the system unmanageable.

  • The contract inherits OpenZeppelin’s Ownable but does not override renounceOwnership(). If the owner calls renounceOwnership(), owner() becomes address(0). All onlyOwner functions (grantSigningRole, revokeSigningRole, proposeTransaction) become permanently inaccessible, effectively bricking administrative control and (given owner‑gated proposals) blocking any future transactions.

contract MultiSigTimelock is Ownable, AccessControl, ReentrancyGuard {
// ...
constructor() Ownable(msg.sender) { // @> Inherits Ownable
// ...
}
function grantSigningRole(address _account)
external
nonReentrant
@> onlyOwner
noneZeroAddress(_account)
{ /* ... */ }
function revokeSigningRole(address _account)
external
nonReentrant
@> onlyOwner
noneZeroAddress(_account)
{ /* ... */ }
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
@> onlyOwner
returns (uint256)
{ /* ... */ }
// @> No override of Ownable.renounceOwnership(), so the owner can zero out ownership.
}

Risk

Likelihood: Low

  • Teams might call renounceOwnership() to “decentralize” or as a safety practice; in this design it removes all admin capabilities with no recovery path.

  • Accidental invocation by scripts or operators (or deliberate action without awareness of consequences) will occur during maintenance or incident response.

Impact: High

  • Permanent loss of admin control: No one can add/remove signers or propose transactions (because proposeTransaction is onlyOwner). If the current signer count is < REQUIRED_CONFIRMATIONS, funds can become locked forever.

  • Incident response impossible: You cannot revoke compromised signers or adjust membership once ownership is renounced.

Proof of Concept

  • Copy the code below to MultiSigTimeLockTest.t.sol.

  • Run command forge test --mt testRenounceOwnershipBricksWallet -vvvv.

function testRenounceOwnershipBricksWallet() public {
// Owner is initially set to deployer; signerCount == 1 (owner is the first signer)
assertEq(multiSigTimelock.owner(), OWNER);
// Add one more signer to show admin actions work pre-renounce
multiSigTimelock.grantSigningRole(SIGNER_TWO);
assertEq(multiSigTimelock.getSignerCount(), 2);
// Owner renounces ownership (sets owner() to address(0))
multiSigTimelock.renounceOwnership();
assertEq(multiSigTimelock.owner(), address(0));
// All onlyOwner functions are now unusable: grant/revoke/propose will revert
vm.expectRevert();
multiSigTimelock.grantSigningRole(SIGNER_THREE);
vm.expectRevert();
multiSigTimelock.revokeSigningRole(SIGNER_TWO);
vm.expectRevert();
multiSigTimelock.proposeTransaction(SPENDER_ONE, 1 ether, hex"");
}

Output:

[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testRenounceOwnershipBricksWallet() (gas: 120948)
Traces:
[128548] MultiSigTimeLockTest::testRenounceOwnershipBricksWallet()
├─ [2537] MultiSigTimelock::owner() [staticcall]
│ └─ ← [Return] MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]
├─ [0] VM::assertEq(MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return]
├─ [81181] MultiSigTimelock::grantSigningRole(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Stop]
├─ [543] MultiSigTimelock::getSignerCount() [staticcall]
│ └─ ← [Return] 2
├─ [0] VM::assertEq(2, 2) [staticcall]
│ └─ ← [Return]
├─ [5298] MultiSigTimelock::renounceOwnership()
│ ├─ emit OwnershipTransferred(previousOwner: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], newOwner: 0x0000000000000000000000000000000000000000)
│ └─ ← [Stop]
├─ [537] MultiSigTimelock::owner() [staticcall]
│ └─ ← [Return] 0x0000000000000000000000000000000000000000
├─ [0] VM::assertEq(0x0000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000) [staticcall]
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [4327] MultiSigTimelock::grantSigningRole(signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])
│ └─ ← [Revert] OwnableUnauthorizedAccount(0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496)
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [4304] MultiSigTimelock::revokeSigningRole(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ └─ ← [Revert] OwnableUnauthorizedAccount(0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496)
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [4789] MultiSigTimelock::proposeTransaction(spender_one: [0x488dE584674d70E8Da70C42Cd4CC73Cd63d3dB23], 1000000000000000000 [1e18], 0x)
│ └─ ← [Revert] OwnableUnauthorizedAccount(0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496)
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.69ms (190.70µs CPU time)
Ran 1 test suite in 16.40ms (2.69ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Disable renouncing ownership.

  • Override renounceOwnership to prevent bricking the wallet:

+ // Add a custom error
+ error MultiSigTimelock__RenounceOwnershipDisabled();
+ function renounceOwnership() public override onlyOwner {
+ revert MultiSigTimelock__RenounceOwnershipDisabled();
+ }

Support

FAQs

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

Give us feedback!