MultiSig Timelock

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

Sending Transactions to the Zero Address is Not Supported

Author Revealed upon completion

Proposing Transactions to the Zero Address is Not Allowed, Preventing the Wallet From Being Able To Send Funds to the Zero Address

Description

  • A standard assumption is that a smart contract wallet should support any actions that can be taken by a standard EOA wallet, in addition to the extra features advertised by the smart contract wallet. The two main selling features of this smart contract wallet are 1) multi-sig and 2) timelock for high value ether transfers. However, this wallet disallows transfers to the zero address, which is not an immediate requirement to support either (1) or (2). Disallowing transfers to the zero address therefore seems unnecessary and adds friction to a potential user adopting this wallet, in the case that that user wanted to make transfers to the zero address.

// Root cause in the codebase with @> marks to highlight the relevant section
function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
@> noneZeroAddress(to)
onlyOwner
returns (uint256)
{

Risk

Likelihood:

  • Transfers to the zero address are not necessarily common, but they do occur. On mainnet ethereum alone, the zero address has over 14.000 Ether as of this finding. Further, there are numerous zero-value transfers to the zero address, which may hold meaning to their callers, even if they appear useless without context. https://etherscan.io/address/0x0000000000000000000000000000000000000000

Impact:

  • Transfers cannot be made to the zero address

Proof of Concept

The following two unit tests show the difference. In the first test, we see that a transfer to the zero address is allowed and can be done by a smart contract. This can also be done by an EOA.

The second test, which was taken from the existing test suite, shows that the MultiSigTimelock disallows this transfer, intentionally.

// This is possible from a standard smart contract, or EOA:
function testCanSendToZeroAddress() public {
vm.deal(address(this), 1 ether);
payable(address(0)).call{value: 1 ether}("");
assertEq(address(0).balance, 1 ether);
}
// but not possible from the MultiSigTimelock
function testProposeTransactionRevertsIfZeroAddress() public {
// ARRANGE
vm.deal(OWNER, OWNER_BALANCE_ONE);
// ACT & ASSERT
vm.prank(OWNER);
vm.expectRevert(MultiSigTimelock.MultiSigTimelock__InvalidAddress.selector);
multiSigTimelock.proposeTransaction(address(0), OWNER_BALANCE_ONE, hex"");
}

Recommended Mitigation

Unless there is a strong reason why transfers to the zero address should not be allowed, they should be supported.

function proposeTransaction(address to, uint256 value, bytes calldata data)
external
nonReentrant
- noneZeroAddress(to)
onlyOwner
returns (uint256)

Support

FAQs

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

Give us feedback!