Description
-
A secure multisig should constrain or validate the size of the calldata forwarded to external contracts (and/or cap the gas forwarded), to prevent gas griefing, memory expansion explosions, and unexpected behavior when interacting with untrusted targets.
-
The wallet stores and forwards arbitrary‑length bytes data and executes it via a low‑level .call() without any bound or per‑target constraint. Extremely large payloads inflate storage and memory costs, can cause out‑of‑gas during execution (especially with memory expansion), and enable griefing or unexpected behavior when calling malicious contracts.
function _proposeTransaction(address to, uint256 value, bytes memory data) internal returns (uint256) {
uint256 transactionId = s_transactionCount;
s_transactions[transactionId] = Transaction({
to: to,
value: value,
data: data,
confirmations: 0,
proposedAt: block.timestamp,
executed: false
});
s_transactionCount++;
emit TransactionProposed(transactionId, to, value);
return transactionId;
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
(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
-
In normal treasury operations, signers often propose data‑only calls (approvals, swaps, staking, governance actions). Without a bound, a signer can submit very large payloads; the proposal will be stored and later executed.
-
Interacting with third‑party contracts (including malicious or upgradable ones) is common; forwarding unbounded data and all gas increases the chance of griefing and non‑deterministic failures.
Impact: Medium
-
Gas griefing / DoS: Huge calldata triggers expensive storage writes on proposal and expensive memory copies on execution, potentially hitting block gas limits or causing repeated OOG when anyone tries to execute.
-
Unexpected behavior: Malicious targets can exploit large input sizes to induce heavy processing, subtle reverts, or pathological memory usage, complicating incident response and monitoring.
Proof of Concept
function testUnboundedDataPayloadAllowsGasGriefing() public {
bytes memory bigData = new bytes(200_000);
uint256 gasBefore = gasleft();
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 0, bigData);
uint256 gasAfter = gasleft();
uint256 gasUsed = gasBefore - gasAfter;
console2.log("Gas used to propose transaction with large data payload:", gasUsed);
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);
gasBefore = gasleft();
multiSigTimelock.executeTransaction(txnId);
gasAfter = gasleft();
gasUsed = gasBefore - gasAfter;
console2.log("Gas used to execute transaction with large data payload:", gasUsed);
}
Output:
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testUnboundedDataPayloadAllowsGasGriefing() (gas: 16461247)
Logs:
Gas used to propose transaction with large data payload: 14833896
Gas used to execute transaction with large data payload: 1252526
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.35ms (13.65ms CPU time)
Ran 1 test suite in 17.99ms (16.35ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
+ // Add an upper bound on calldata size
+ uint256 private constant MAX_DATA_LENGTH = 4096; // e.g., 4 KB
+ // Custom error
+ error MultiSigTimelock__DataTooLong(uint256 length, uint256 max);
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
noneZeroAddress(to)
onlyOwner
returns (uint256)
{
+ // Enforce size limit at proposal time to prevent storing huge blobs on-chain
+ if (data.length > MAX_DATA_LENGTH) {
+ revert MultiSigTimelock__DataTooLong(data.length, MAX_DATA_LENGTH);
+ }
return _proposeTransaction(to, value, data);
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
+ // Defensive check at execution time (covers legacy stored txns or future policy changes)
+ if (txn.data.length > MAX_DATA_LENGTH) {
+ revert MultiSigTimelock__DataTooLong(txn.data.length, MAX_DATA_LENGTH);
+ }
- (bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
+ // Optional: cap gas forwarded to the target to limit griefing (tune carefully)
+ uint256 EXEC_GAS_LIMIT = 500_000; // example value; ensure legitimate calls still fit
+ (bool success,) = payable(txn.to).call{value: txn.value, gas: EXEC_GAS_LIMIT}(txn.data);
if (!success) { revert MultiSigTimelock__ExecutionFailed(); }
emit TransactionExecuted(txnId, txn.to, txn.value);
}
