Skip to content

Instantly share code, notes, and snippets.

@Dexaran
Last active July 24, 2023 15:54
Show Gist options
  • Star 5 You must be signed in to star a gist
  • Fork 4 You must be signed in to fork a gist
  • Save Dexaran/2389d5e7290ab69709d33abfe0485bec to your computer and use it in GitHub Desktop.
Save Dexaran/2389d5e7290ab69709d33abfe0485bec to your computer and use it in GitHub Desktop.
Ethereum CLassic multisig wallet audit report.

Ethereum Classic Multisig Wallet audit report.

Summary

This is the report from a security audit performed on Ethereum Classic Multisig Wallet by Dexaran. This contracts are a version of OpenZeppelin Multisig Wallet.

The audit focused primarily on the security of funds and fault tolerance of the multisig wallet. The main intention of this multisig wallet is to serve as an official storage of funds donated for ETC development.

In scope

  1. MultisigWallet.sol

  2. DayLimit.sol

  3. Multisig.sol

  4. Shareable.sol

Findings

In total, 7 issues were reported including:

  • 2 medium severity issues.

  • 4 low severity issues.

  • 1 minor observation.

No critical security issues were found.

Security issues

1. Permanent ownership.

Severity: medium

Description

Each owner of the multisig wallet is permanent. A mechanism to update a list of owners or claim ownership is not presented. This can result in a permanent loss of funds in some specific circumstances.

Detailed description can be found here.

Recommendation

Implement a mechanism to manage ownership/ claim ownership.

2. Interface function definitions caused Remix compiler error.

Severity: medium

Description

The definitions of the functions of Multisig.sol contract made the Remix compiler unable of generating bytecode of the contract.

This is not a security issue, but a code flaw. This can not affect a deployed contract, but this made compiler behave in unintended way. This hurts contract's reusability property as well.

Detailed description can be found here.

Recommendation

Change the implementation of empty function definitions.

3. Permissive fallback function.

Severity: low

Description

The fallback function allows any third-party contracts to send calls to a Multisig Wallet contract without any restrictions.

This can not directly affect Multisig wallet state or funds. This can only hurt third-party contracts that will interact with the Multisig Wallet contract.

Detailed description can be found here.

4. Inner variables visibility.

Severity: low

Description

Some internal contract variables are hidden. This does not add any security, but can make it difficult to find errors if they occur.

Detailed description can be found here.

Recommendation

Specify public visibility for the contract's inner variables.

5. External parameter of a function shadows an existing declaration.

Severity: low

Description

getOwner(uint256 ownerIndex) function of Shareable.sol contract has a parameter that shadows an existing declaration. It is likely a mistyped parameter name.

It is strongly recommended to adhere to the same coding standard when developing important components of blockchain systems such as multisig wallet. Temporary variables and parameters must have a _ prefix in this case.

Detailed description can be found here.

Recommendation

Rename uint256 ownerIndex parameter to uint256 _ownerIndex.

6. Missing function visibility specifier.

Severity: low

Description

confirm(bytes32 _h) function of Multisig.sol contract has no visibility specifier unlike the other functions of this contract. It is strongly recommended to adhere to the same coding standard when developing important components of blockchain systems such as multisig wallet.

Detailed description can be found here.

Recommendation

Implement a public visibility specifier for confirm function of Multisig.sol and MultisigWallet.sol contracts.

7. ERC223 and ERC777 compatibility issue.

Severity: not a security issue

Description

This wallet does not implement ERC223 receiver or ERC777 receiver interface.

It will still accept token transfers from contracts that implement ERC223 or ERC777 token standards but it will lead to a silent contract execution without proper event logging.

Specification

The contracts at EthereumCommonwealth/ethereum-classic-multisig repo are a copy of contracts from the Open Zeppelin repository, with the exclusion of contracts that are not related to a multisig wallet.

These contracts are in ready to use condition. At the time of the audit, the development of this contracts is considered complete. The last fixation was made on July 18, 2017. the last commit was made at 18 Jul 2017: https://github.com/OpenZeppelin/zeppelin-solidity/commit/4f444279661e7c2e6ff7c442c3d13ae45d33b892

Bug Bounties were not conducted.

Any further changes to the contracts will leave them in unaudited state.

Conclusion

No critical vulnerabilities were detected. The reported issues can not directly hurt the Multisig smart-contract. The multisig smart-contracts can satisfy the main goal and could be used for official development donations storage.

It is highly recommended to complete a bug bounty before use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment