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

Self-call vulnerability

Author Revealed upon completion

Description

  • A multisig wallet should not be able to execute arbitrary calls back into itself. Self‑calls can change how msg.sender is perceived (becoming the contract address), complicate access control assumptions, and expand the reentrancy surface.

  • he contract permits transactions where to == address(this). During execution, it performs a low‑level .call() with arbitrary data, allowing self‑calls. While many external functions are guarded (onlyOwner, onlyRole(SIGNING_ROLE), and nonReentrant), the lack of a hard prohibition on self‑calls creates pathways for misuse:

    • If the contract address is ever (accidentally) granted SIGNING_ROLE, self‑invoked calls would see msg.sender == address(this) and pass role checks.

    • Future code changes (e.g., adding an unguarded external function or removing nonReentrant) could immediately make self‑calls a privilege escalation vector.

    • Even today, self‑calls can be used for gas griefing or unexpected control flow (e.g., triggering receive() with large value and data paths), increasing operational risk.

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
// ... quorum & timelock checks ...
​
// @> No guard against self-calls; 'to' can be address(this)
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) { revert MultiSigTimelock__ExecutionFailed(); }
​
emit TransactionExecuted(txnId, txn.to, txn.value);
}

Risk

Likelihood: Medium

  • During routine operations, admins may mistakenly grant SIGNING_ROLE to the contract address (allowed by grantSigningRole), or future refactors may add new external functions without proper guards - both make self‑calls plausible and dangerous.

  • Teams often craft complex data payloads; allowing self‑targeting increases the chance of unexpected interactions and reentrancy‑adjacent behaviors.

Impact: Medium

  • Privilege escalation & state manipulation: If the contract address holds a privileged role (or a new unguarded function is introduced), a self‑call can grant/revoke roles, propose, confirm, or execute transactions without the intended human gatekeeping.

  • Operational instability: Self‑calls enlarge the attack surface for griefing/DoS, confuse monitoring (events look like internal activity), and undermine the clarity of who initiated sensitive changes.

Proof of Concept

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

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

function testSelfCallIsAcceptedAndExecutable() public {
// 1) Propose a transaction targeting the contract itself.
// This is accepted today; there is no guard against 'to == address(this)'.
uint256 txnId = multiSigTimelock.proposeTransaction(
address(multiSigTimelock),
0, // value 0 to bypass ETH-based timelock tiers
abi.encodeWithSignature("getSignerCount()") // arbitrary call back into the contract
);
​
// 2) Reach quorum (owner + two more signers)
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
​
// 3) Execute: this performs address(this).call(...), proving self-calls are permitted.
// (This specific payload is benign, but demonstrates the missing guard.)
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txnId);
}

Output:

[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testSelfCallIsAcceptedAndExecutable() (gas: 402812)
Traces:
[422412] MultiSigTimeLockTest::testSelfCallIsAcceptedAndExecutable()
├─ [107470] MultiSigTimelock::proposeTransaction(MultiSigTimelock: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 0, 0xb715be81)
│ ├─ emit TransactionProposed(transactionId: 0, to: MultiSigTimelock: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 0)
│ └─ ← [Return] 0
├─ [79181] MultiSigTimelock::grantSigningRole(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Stop]
├─ [74381] MultiSigTimelock::grantSigningRole(signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])
│ ├─ emit RoleGranted(role: 0x8d12c52fe7c286acb23e8b6fad43ba0a7b1bdee59ad6818c044e478cc487c15b, account: signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428], sender: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Stop]
├─ [51353] MultiSigTimelock::confirmTransaction(0)
│ ├─ emit TransactionConfirmed(transactionId: 0, signer: MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Stop]
├─ [0] VM::prank(signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ └─ ← [Return]
├─ [29453] MultiSigTimelock::confirmTransaction(0)
│ ├─ emit TransactionConfirmed(transactionId: 0, signer: signer_two: [0x884D93fF97A786C23d91993E20382Fc4C26Bc2aa])
│ └─ ← [Stop]
├─ [0] VM::prank(signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])
│ └─ ← [Return]
├─ [29453] MultiSigTimelock::confirmTransaction(0)
│ ├─ emit TransactionConfirmed(transactionId: 0, signer: signer_three: [0x6d3780bE9713626035Ad76FfD17fCDc3FfD29428])
│ └─ ← [Stop]
├─ [0] VM::prank(MultiSigTimeLockTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [29736] MultiSigTimelock::executeTransaction(0)
│ ├─ [543] MultiSigTimelock::getSignerCount()
│ │ └─ ← [Return] 3
│ ├─ emit TransactionExecuted(transactionId: 0, to: MultiSigTimelock: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 0)
│ └─ ← [Stop]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.70ms (344.70µs CPU time)
​
Ran 1 test suite in 11.82ms (2.70ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Explicitly forbid self‑calls and prevent assigning roles to the contract address:

​
+ error MultiSigTimelock__SelfCallForbidden();
​
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
onlyOwner
returns (uint256)
{
+ // Disallow proposals that target the wallet itself
+ if (to == address(this)) {
+ revert MultiSigTimelock__SelfCallForbidden();
+ }
return _proposeTransaction(to, value, data);
}
​
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
+ // Defensive check at execution time as well
+ if (txn.to == address(this)) {
+ revert MultiSigTimelock__SelfCallForbidden();
+ }
// ... remainder unchanged ...
}
​

Support

FAQs

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

Give us feedback!