MultiSigWallet.md

Ethereum Classic MultiSigWallet smart contract audit report.

Summary

This is the report from a security audit performed on MultiSigWallet smart contract (ETH) by alexo18. The audit focused primarily on the security of funds and fault tolerance of the MultiSigWallet contract. The contract allows multiple parties to agree on transactions before execution.

In scope

[MultiSigWalletWithDailyLimit]https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol)

Findings

In total, 4 issues were reported including:

• 3 medium severity issues.
• 1 low severity issues.

Security issues

1. Input parameters validity.

Severity: medium

Description

The 'addTransaction' function does not check for the validity of the incoming 'value' parameter for positivity.
https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L263

The 'replaceOwner' function does not check for the validity of the incoming 'newOwner' parameter for zero address.
In some specific circumstances it can lead to situation when the ' _required > ownerCount' criteria never met. https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L154

The 'getTransactionIds' function does not check for the validity of the incoming 'from' and 'to' parameters (negative 'to' - 'from' result).
https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L347

2. Gas consumption.

Severity: medium

Description

The 'ExecuteTransaction' function does not put a lid on gas consumption. Given the unsafe nature of the "address.call.value()()" method , certain called functions can deplete the gas.

Code snippet

https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L222 https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L405

Recommendation

Limit the gas usage to a logically acceptable top border value.

3.Code mistake.

Severity: medium

Description

The 'getTransactionIds' function apply the 'from' and 'to' parameters to the 'transactionIdsTemp' array , which may be differently sized than the 'transaction' array .

Code snippet

https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L364

Recommendation

Apply the 'from' , 'to' parameters to the 'transaction' array instead of the 'transactionIdsTemp' array.

4. SafeMath not used in math operations.

Severity: low

Code snippet

https://github.com/ConsenSys/MultiSigWallet/blob/master/MultiSigWalletWithDailyLimit.sol#L440

Recommendation

Use SafeMath

Conclusion

No critical vulnerabilities were detected,bu we highly recommend to complete other bugs before use.