The LAO

1 Summary

ConsenSys Diligence conducted a security audit on Moloch DAO v2 (The LAO, MolochLAO) in January 2020. Moloch DAO v2, is similar to Moloch DAO v1 with additional features such as: Multi-Token Support, Tokens Whitelisting, Emergency Processing, Guild Kick, non-voting shares (Loot), and many others (Full change log can be found here).

2 Audit Scope

This audit covered the following files, based on the frozen commit 4bc443f:

UPDATE: Due to the fact that most of the found issues had a similar remediation, Moloch team decided to start the fixes, in pull-pattern branch, earlier than the scheduled mitigation phase. We are working with Moloch team to review most of the fixes (mainly on GitHub and Telegram chat), and will try to include as much as possible in this report. It should be noted that the system summary, workflow charts, and tool-based analysis were done based on the original commit hash prior to the redesign, and partly updated to reflect the new changes. These changes include but not limited to merging Moloch.sol and GuildBank.sol into one contract, and using Pull patterns for withdrawal rather push for fund transfers.

The final commit hash reviewed in this report is pull-pattern/72de553.

The audit activities can be grouped into the following three broad categories:

1. Security: Identifying security related issues within the contract.
2. Architecture: Evaluating the system architecture through the lens of established smart contract best practices.
3. Code quality: A full review of the contract source code. The primary areas of focus include:
   • Correctness
   • Readability
   • Scalability
   • Code complexity
   • Quality of test coverage

2.1 Documentation

The following documentation was available to the project team:

• Inline code documentation
• The LAO—Moloch v2 Smart Contract Audit Kickoff Call
• Documentation
• MetaCartel Ventures Whitepaper

3 System Overview

3.1 Detailed Design

MolochDAO consists of two main smart contracts:

• Moloch
• GuildBank

The Actors and their permissions in the system are described in Security specification section. The following figure is a visualization of the actors and the overview of Moloch DAO functionality before the start of the audit:

Update: The following is the system overview visualization of the DAO after the audit. As seen in the chart, changes are resulted in a much simpler codebase and workflow. Some of these changes are as following:

• Removal of the GuildBank smart contract
• Removal of bailOut() , safeRagequit, Emergency Processing
• Replacing most of the external calls for ERC20 transfers with internal token balance changes (userTokenBalances)
• Replacing external ERC20 transfers within the pull pattern (Withdraw)

Proposal in MolochDAO

Proposals in the MolochDAO use a state machine, which depending on the type of proposal can fall into different states. They must pass requirements, mainly specified in Moloch._validateProposalForProcessing() and Moloch._didPass() functions, to be processed. The following sequence diagram shows the overall lifecycle of the proposals:

4 Security Specification

This section describes, from a security perspective, the expected behavior of the system under audit. It is not a substitute for documentation. The purpose of this section is to identify specific security properties that were validated by the audit team.

4.1 Actors

The relevant actors are as follows:

• Summoner

  • Specified by the actor deploying the DAO
  • The sole initial member and share holder (1 share)
  • The summoner cannot be GuildKick
  • Receives loots on all bailouts

• Member

  • The member can sponsor proposals
    • These proposals can be any of the following:
      • Add new member
      • Whitelist Tokens
      • Guild Kick other members
      • Ask for more share/loots in a proposal
      • Make a payment (/trade)
      • Ceremonial Votes (details field for off-chain decision)
  • Can rage quit from the DAO and withdraw her funds
  • Can assign a secondary delegate key for her membership
  • Can Vote on proposals

• Anyone

  • Can submit a new proposal to the DAO
  • Can cancel their own proposals
  • Can rage kick jailed members (given that the requirements are satisfied)
  • Can bail out jailed members (given that the requirements are satisfied)
  • Can call to process proposals (given that the requirements are satisfied)
  • Withdraw their token balance in the DAO

4.2 Trust Model

MolochDAO uses a social approach to mitigate many of the common decentralize attacks on anonymous stake-based DAOs. Membership Admission is Permissioned, The new member proposals are only sponsored by current members, hence any member of the DAO should be known by at least one of the existing members to join.

Continuing Membership is Permissioned and Community-Policed, meaning that any member can quit the DAO (ragequit) or can propose another member to be kicked out of the DAO (ragekick), conditioned to votes from sufficient members.

The overall motto of Moloch DAO is that false negatives are better than false positives. Meaning that if for any reason something is going wrong with a proposal, it’s better to fail and refund all the participants than to accept a faulty proposal and stake members’ funds. This has been implied by the functionalities such as ragequit, that if a DAO does not follow ones ideology, they are free to quit the DAO and get a refund.

Most of the trust model and assumptions of the Moloch DAO are well documented in the MetaCartel Ventures Whitepaper.

4.3 Security Considerations

The following is a non-exhaustive list of security considerations:

• Summoner is the sole member in the start of the DAO and is responsible to sponsor new member proposals.
• periodDuration is simply the unit of time in Moloch DAO. It is defaulted to 4.8 hours (can be changed in deploy time only), meaning there are 5 periods per day. This period is used to determine time length in the DAO (e.g. votingPeriod = 35 * periodDuration which is 7 days, same for gracePeriodLength). These two periods would mitigate most of the possible attacks (e.g. Burning man attack), however, given enough incentive to attack the DAO, the attacker could potentially congest the network for one period (4.8 hours), and prevent members from rage quiting before the grace period. This attack is really unlikely to happen due to high cost of performing this attack and also considering the social aspect of the DAO that each member is sponsored by another existing member.
• Tribute Token transfers for 0 amount is possible where the proposal does not include a payment but needs to be passed. This can be optimized by a check for the amount before the transfer.

5 Issues

Each issue has an assigned severity:

• Minor issues are subjective in nature. They are typically suggestions around best practices or readability. Code maintainers should use their own judgment as to whether to address such issues.
• Medium issues are objective in nature but are not security vulnerabilities. These should be addressed unless there is a clear reason not to.
• Major issues are security vulnerabilities that may not be directly exploitable or may require certain conditions in order to be exploited. All major issues should be addressed.
• Critical issues are directly exploitable security vulnerabilities that need to be fixed.

if (!emergencyProcessing) {
    require(
        proposal.tributeToken.transfer(proposal.proposer, proposal.tributeOffered),
        "failing vote token transfer failed"
    );

if (!emergencyProcessing) {
    require(
        proposal.tributeToken.transfer(proposal.proposer, proposal.tributeOffered),
        "failing vote token transfer failed"
    );

function max(uint256 x, uint256 y) internal pure returns (uint256) {
    return x >= y ? x : y;
}

uint256 constant MAX_TOKEN_WHITELIST_COUNT = 400; // maximum number of whitelisted tokens
uint256 constant MAX_TOKEN_GUILDBANK_COUNT = 200; // maximum number of tokens with non-zero balance in guildbank
uint256 public totalGuildBankTokens = 0; // total tokens with non-zero balance in guild bank

for (uint256 i = 0; i < tokens.length; i++) {
    uint256 amountToRagequit = fairShare(userTokenBalances[GUILD][tokens[i]], sharesAndLootToBurn, initialTotalSharesAndLoot);
    // deliberately not using safemath here to keep overflows from preventing the function execution (which would break ragekicks)
    // if a token overflows, it is because the supply was artificially inflated to oblivion, so we probably don't care about it anyways
    userTokenBalances[GUILD][tokens[i]] -= amountToRagequit;
    userTokenBalances[memberAddress][tokens[i]] += amountToRagequit;
}

// whitelist proposal
if (proposal.flags[4]) {
    require(!tokenWhitelist[address(proposal.tributeToken)], "cannot already have whitelisted the token");
    require(!proposedToWhitelist[address(proposal.tributeToken)], 'already proposed to whitelist');
    proposedToWhitelist[address(proposal.tributeToken)] = true;

bool[6] flags; // [sponsored, processed, didPass, cancelled, whitelist, guildkick]

        bool[6] memory flags; // [sponsored, processed, didPass, cancelled, whitelist, guildkick]

pragma solidity 0.5.15;

contract FlagsEnum {
    struct Proposal {
        address applicant;
        uint value;
        bool[3] flags; // [sponsored, processed, kicked]
    }
   
    enum ProposalFlags {
        SPONSORED,
        PROCESSED,
        KICKED
    }
   
    uint proposalCount;
   
    mapping(uint256 => Proposal) public proposals;
   
    function addProposal(uint _value, bool _sponsored, bool _processed, bool _kicked) public returns (uint) {
        Proposal memory proposal = Proposal({
            applicant: msg.sender,
            value: _value,
            flags: [_sponsored, _processed, _kicked]
        });
       
        proposals[proposalCount] = proposal;
        proposalCount += 1;
       
        return (proposalCount);
    }
   
    function getProposal(uint _proposalId) public view returns (address, uint, bool, bool, bool) {
        return (
            proposals[_proposalId].applicant,
            proposals[_proposalId].value,
            proposals[_proposalId].flags[uint(ProposalFlags.SPONSORED)],
            proposals[_proposalId].flags[uint(ProposalFlags.PROCESSED)],
            proposals[_proposalId].flags[uint(ProposalFlags.KICKED)]
        );
    }
}

6 Tool-Based Analysis

Several tools were used to perform automated analysis of the reviewed contracts. These issues were reviewed by the audit team, and relevant issues are listed in the Issue Details section.

6.1 MythX

MythX is a security analysis API for Ethereum smart contracts. It performs multiple types of analysis, including fuzzing and symbolic execution, to detect many common vulnerability types. The tool was used for automated vulnerability discovery for all audited contracts and libraries. More details on MythX can be found at mythx.io.

6.2 Ethlint

Ethlint is an open source project for linting Solidity code. Only security-related issues were reviewed by the audit team.

Below is the raw output of the Ethlint vulnerability scan:

contracts/GuildBank.sol
  13:8     warning    Provide an error message for require()                                                     error-reason
  23:12    warning    Provide an error message for require()                                                     error-reason
  34:8     warning    Provide an error message for require()                                                     error-reason
  36:27    warning    There should be no whitespace or comments between the opening brace '{' and first item.    whitespace
  36:37    warning    There should be no whitespace or comments between the last item and closing brace '}'.     whitespace

contracts/Moloch.sol
  34:4      warning    Line exceeds the limit of 145 characters                                   max-len
  41:4      warning    Line exceeds the limit of 145 characters                                   max-len
  42:4      warning    Line exceeds the limit of 145 characters                                   max-len
  76:8      warning    Line exceeds the limit of 145 characters                                   max-len
  169:24    warning    Avoid using 'now' (alias to 'block.timestamp').                            security/no-block-members
  262:8     warning    Line exceeds the limit of 145 characters                                   max-len
  272:49    error      String literal must be quoted with double quotes.                          quotes
  280:74    error      String literal must be quoted with double quotes.                          quotes
  285:57    error      String literal must be quoted with double quotes.                          quotes
  362:8     warning    Line exceeds the limit of 145 characters                                   max-len
  517:8     warning    Line exceeds the limit of 145 characters                                   max-len
  540:13    warning    Assignment operator must have exactly single space on both sides of it.    operator-whitespace
  559:8     warning    Error message exceeds max length of 76 characters                          error-reason
  583:8     warning    Error message exceeds max length of 76 characters                          error-reason
  641:15    warning    Avoid using 'now' (alias to 'block.timestamp').                            security/no-block-members

contracts/oz/ERC20.sol
  128:8    warning    Provide an error message for require()    error-reason
  143:8    warning    Provide an error message for require()    error-reason
  157:8    warning    Provide an error message for require()    error-reason
  171:8    warning    Provide an error message for require()    error-reason
  172:8    warning    Provide an error message for require()    error-reason

contracts/oz/SafeMath.sol
  10:8    warning    Provide an error message for require()    error-reason
  17:8    warning    Provide an error message for require()    error-reason
  24:8    warning    Provide an error message for require()    error-reason
  32:8    warning    Provide an error message for require()    error-reason

contracts/test-helpers/Submitter.sol
  9:2     error    Only use indent of 4 spaces.    indentation
  11:2    error    Only use indent of 4 spaces.    indentation
  13:2    error    Only use indent of 4 spaces.    indentation
  15:0    error    Only use indent of 4 spaces.    indentation
  17:2    error    Only use indent of 4 spaces.    indentation
  39:0    error    Only use indent of 4 spaces.    indentation
  41:2    error    Only use indent of 4 spaces.    indentation
  51:0    error    Only use indent of 4 spaces.    indentation
  53:2    error    Only use indent of 4 spaces.    indentation
  63:0    error    Only use indent of 4 spaces.    indentation

contracts/tokens/ClaimsToken.sol
  95:1     error      Only use indent of 4 spaces.              indentation
  98:1     error      Only use indent of 4 spaces.              indentation
  100:1    error      Only use indent of 4 spaces.              indentation
  102:1    error      Only use indent of 4 spaces.              indentation
  105:1    error      Only use indent of 4 spaces.              indentation
  112:0    error      Only use indent of 4 spaces.              indentation
  121:1    error      Only use indent of 4 spaces.              indentation
  129:0    error      Only use indent of 4 spaces.              indentation
  140:1    error      Only use indent of 4 spaces.              indentation
  148:0    error      Only use indent of 4 spaces.              indentation
  154:1    error      Only use indent of 4 spaces.              indentation
  160:0    error      Only use indent of 4 spaces.              indentation
  167:1    error      Only use indent of 4 spaces.              indentation
  173:0    error      Only use indent of 4 spaces.              indentation
  180:1    error      Only use indent of 4 spaces.              indentation
  184:0    error      Only use indent of 4 spaces.              indentation
  190:1    error      Only use indent of 4 spaces.              indentation
  197:0    error      Only use indent of 4 spaces.              indentation
  203:1    error      Only use indent of 4 spaces.              indentation
  208:0    error      Only use indent of 4 spaces.              indentation
  216:1    error      Only use indent of 4 spaces.              indentation
  226:0    error      Only use indent of 4 spaces.              indentation
  232:1    error      Only use indent of 4 spaces.              indentation
  234:1    error      Only use indent of 4 spaces.              indentation
  237:0    error      Only use indent of 4 spaces.              indentation
  239:1    error      Only use indent of 4 spaces.              indentation
  243:2    warning    Provide an error message for require()    error-reason
  246:0    error      Only use indent of 4 spaces.              indentation
  251:1    error      Only use indent of 4 spaces.              indentation
  260:0    error      Only use indent of 4 spaces.              indentation
  268:1    error      Only use indent of 4 spaces.              indentation
  276:0    error      Only use indent of 4 spaces.              indentation

contracts/tokens/Token.sol
  25:8    warning    Provide an error message for require()    error-reason

✖ 44 errors, 28 warnings found.

6.3 Surya

Surya is a utility tool for smart contract systems. It provides a number of visual outputs and information about the structure of smart contracts. It also supports querying the function call graph in multiple ways to aid in the manual inspection and control flow analysis of contracts.

Below is a complete list of functions with their visibility and modifiers:

Files Description Table

Contracts Description Table

Legend

Appendix 1 - Disclosure

ConsenSys Diligence (“CD”) typically receives compensation from one or more clients (the “Clients”) for performing the analysis contained in these reports (the “Reports”). The Reports may be distributed through other means, including via ConsenSys publications and other distributions.

The Reports are not an endorsement or indictment of any particular project or team, and the Reports do not guarantee the security of any particular project. This Report does not consider, and should not be interpreted as considering or having any bearing on, the potential economics of a token, token sale or any other product, service or other asset. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty. No Report provides any warranty or representation to any Third-Party in any respect, including regarding the bugfree nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third party should rely on the Reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this Report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project. CD owes no duty to any Third-Party by virtue of publishing these Reports.

PURPOSE OF REPORTS The Reports and the analysis described therein are created solely for Clients and published with their consent. The scope of our review is limited to a review of Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty.

CD makes the Reports available to parties other than the Clients (i.e., “third parties”) – on its website. CD hopes that by making these analyses publicly available, it can help the blockchain ecosystem develop technical best practices in this rapidly evolving area of innovation.

LINKS TO OTHER WEB SITES FROM THIS WEB SITE You may, through hypertext or other computer links, gain access to web sites operated by persons other than ConsenSys and CD. Such hyperlinks are provided for your reference and convenience only, and are the exclusive responsibility of such web sites’ owners. You agree that ConsenSys and CD are not responsible for the content or operation of such Web sites, and that ConsenSys and CD shall have no liability to you or any other person or entity for the use of third party Web sites. Except as described below, a hyperlink from this web Site to another web site does not imply or mean that ConsenSys and CD endorses the content on that Web site or the operator or operations of that site. You are solely responsible for determining the extent to which you may use any content at any other web sites to which you link from the Reports. ConsenSys and CD assumes no responsibility for the use of third party software on the Web Site and shall have no liability whatsoever to any person or entity for the accuracy or completeness of any outcome generated by such software.

TIMELINESS OF CONTENT The content contained in the Reports is current as of the date appearing on the Report and is subject to change without notice. Unless indicated otherwise, by ConsenSys and CD.