Polymath-Core Audit

• 1 Summary
  • 1.1 Audit Dashboard
  • 1.2 Audit Goals
  • 1.3 System Overview
  • 1.4 Key Observations/Recommendations
• 2 Issue Overview
• 3 Issue Details
  • 3.1 SecurityToken contract should always be initialized
  • 3.2 Unpredictable behavior due to front running or general bad timing
  • 3.3 Polymath can arbitrarily change prices during a USDTieredSTO token sale
  • 3.4 No new ST can be created after implementation upgrade
  • 3.5 ModuleRegistry - Custom modules can block their own removal
  • 3.6 Transfer decisions by default should be consistent
  • 3.7 ModuleRegistry - Custom module verification should be bounded to module version
  • 3.8 ModuleRegistry - Risks of allowing custom modules in the system
  • 3.9 _balanceOfByPartition function returns wrong value
  • 3.10 _returnPartition function of SecurityToken always returns UNLOCKED partition
  • 3.11 partitionsOf function always returns empty array
  • 3.12 Security token upgrade info should be editable
  • 3.13 VestingEscrowWallet - Integer Underflow and unchecked array access in pushAvailableTokensMulti()
  • 3.14 SecurityTokenRegistry does not inherit from ISecurityTokenRegistry
  • 3.15 Different implementations for the same modifier whenNotPausedOrOwner
  • 3.16 SecurityToken - security token name change may cause inconsistency
  • 3.17 Where possible, a specific contract type should be used rather than address
  • 3.18 SecurityToken - Missing Input Validation changeName
  • 3.19 deleteDelegate should be implemented without array iteration in permission manager
  • 3.20 Improve Code Reusability - Use const variables instead of literals for EIP-1066 status codes.
  • 3.21 TokenLib - Unused Import KindMath
  • 3.22 Module - authentication modifier onlyFactoryOwner and onlyFactoryOrOwner are never used
  • 3.23 SecurityToken - authentication modifier onlyTokenFactory is never used
  • 3.24 SecurityToken/STGetter (ERC1643) getAllDocuments - can be cause gas or memory issues if used
  • 3.25 PolyToken - events are redefined in the implementation
  • 3.26 Use latest stable version of Solidity
  • 3.27 OwnedProxy should be removed
  • 3.28 Module.takeUsageFee() allows admins to drain a module's approved POLY tokens
  • 3.29 Redundant pause and unpause functions in TransferManager
  • 3.30 Modules shouldn't be casted to IBoot
  • 3.31 public functions in TokenLib could be external instead
  • 3.32 EtherDividendCheckpoint and ERC20DividendCheckpoint division by zero
  • 3.33 SecurityToken / STGetter storage layout is hard to maintain
  • 3.34 LockUpTransferManager._removeLockUpFromUser checks for an impossible condition
• 4 Threat Model
  • 4.1 Overview
  • 4.2 Security Stories
• 5 Tool-Based Analysis
  • 5.1 Mythril
  • 5.2 Harvey
  • 5.3 Ethlint
  • 5.4 Surya
• 6 Test Coverage Measurement
  • 6.1 Test Coverage observations
• 7 Business Logic Review v2.2.0 to v3.0.0 Modules
• Appendix 1 - File Hashes
• Appendix 2 - Severity
  • A.2.2 - Minor
  • A.2.3 - Medium
  • A.2.4 - Major
  • A.2.5 - Critical
• Appendix 3 - Disclosure

1 Summary

ConsenSys Diligence conducted a security audit on the Polymath Core smart contracts that comprise a system for launching regulatory-compliant securities tokens on a decentralized blockchain.

1.1 Audit Dashboard

Audit Details

• Project Name: Polymath-Core
• Client Name: Polymath Inc.
• Client Contact: Adam Dossa
• Lead Auditor: Martin Ortner
• Co-auditors: Sergii Kravchenko, Steve Marx
• Date: 13 May 2019
• Commit Hash: #7a70aa5 (tag/3.0.0-audit)
• Repository: https://github.com/PolymathNetwork/polymath-core/releases/tag/3.0.0-audit

Number of issues by severity

1.2 Audit Goals

The focus of the audit was to verify that the smart contract system is secure, resilient and working according to its specifications. The audit activities can be grouped in the following three categories:

Security: Identifying security related issues within each contract and within the system of contracts.

Sound Architecture: Evaluation of the architecture of this system through the lens of established smart contract best practices and general software best practices.

Code Correctness and Quality: A full review of the contract source code. The primary areas of focus include:

• Correctness
• Readability
• Sections of code with high complexity
• Improving scalability
• Quantity and quality of test coverage

1.3 System Overview

Documentation

The following documentation was available to the audit team:

• Documentation
• Core Specification

Detail

This section outlines the top-level contracts and libraries of the system. A summary of the inheritance structure for the main component groups is given in the Design section. A complete view of the inheritance structure can be found here (dot). The complete call-graph is too large to be rendered as an image and is therefore only provided as a graphviz dot file.

Registries

• PolymathRegistry
• FeatureRegistry
• ModuleRegistry and ModuleRegistryProxy
• SecurityTokenRegistry, STRGetter and SecurityTokenRegistryProxy

Oracle Interfaces

• MakerDAOOracle
• StableOracle
• PolyOracle

SecurityToken

• SecurityTokenFactory (STFactory)
  • SecurityToken, STGetter and SecurityTokenProxy

Modules

CheckPoint

• ERC20DividendCheckpoint, -Factory and -Proxy
• EtherDividendCheckpoint, -Factory and -Proxy
• WeightedVoteCheckpoint, -Factory and -Proxy
• PLCRVotingCheckpoint, -Factory and -Proxy

SecurityTokenOffering (STO)

• CappedSTO, -Factory, -Storage and -Proxy
• USDTieredSTO, -Factory, -Storage and -Proxy
• PresaleSTO, -Factory, -Storage and -Proxy

TransferManager

• BlacklistTransferManager, -Factory, -Storage and -Proxy
• CountTransferManager, -Factory, -Storage and -Proxy
• GeneralTransferManager, -Factory, -Storage and -Proxy
• LockupTransferManager, -Factory, -Storage and -Proxy
• ManualApprovalTransferManager, -Factory, -Storage and -Proxy
• PercentageTransferManager, -Factory, -Storage and -Proxy
• VolumeRestrictionTransferManager, -Factory, -Storage and -Proxy

Wallet

• VestingEscrowWallet, -Factory, -Storage and -Proxy

PermissionManager

• GeneralPermissionManager, -Factory, -Storage and -Proxy

Scope

The scope of the audit was defined as the Solidity code in the ./contracts/ folder except

• Experimental modules (contracts/modules/Experimental)
• Migrations
• Mocks (contracts/mocks)
• External (contracts/external)
• Helpers (contracts/helpers)

The list of files and their hashes can be found in Appendix 1 - File Hashes.

In addition to the v3.0.0 audit the team also reviewed the three v2.2.0 modules to match their v3.0.0 counterparts. Details can be found in the section Business Logic Review v2.2.0 to v3.0.0 Modules.

Design

The system is a framework to launch and manage security tokens. An end user typically creates a new security token by registering a token symbol and deploying a security token contract. A security token can make use of one or more modules that specify the behavior of the token.

The system consists of three major component groups:

• Registries

  _(dot)

  Registries are administered and deployed by Polymath and form the backbone of the system. There are four Registries:

  • SecurityTokenRegistry

    The SecurityTokenRegistry is the main entry point to the protocol and allows token issuers to register symbols and generate new security tokens. It keeps track of all security tokens and links issuers to security tokens and the symbols they reserved.

    The SecurityTokenRegistry is proxied and can be upgraded. Ownership remains with Polymath.

    Main Functionality:

    • manage symbols: create, modify, and remove tickers and transfer their ownership
    • manage security tokens: generate and modify security tokens
    • manage fees: handle fees for ticker registration and security token launch

  • ModuleRegistry

    The ModuleRegistry keeps track of all registered modules that are available to security tokens and provides access to the module factories.

    The ModuleRegistry is proxied and can be upgraded. Ownership remains with Polymath.

    Main Functionality:

    • manage modules: register, remove, verify, unverify, and access modules

  • FeatureRegistry

    The FeatureRegistry supports enabling and disabling features in the system. It maps a feature name to a boolean value indicating whether that feature is enabled. In the audited code, the only feature tracked is called customModulesAllowed. This feature is checked by the ModuleRegistry to see whether custom modules (those not managed by Polymath) should be allowed.

    The FeatureRegistry is not proxied and cannot be upgraded. Ownership remains with Polymath.

    Main Functionality:

    • manage features: globally enable and disable features

  • PolymathRegistry

    The PolymathRegistry is the main registry that keeps track of core components of the system including the addresses of the PolyToken (e.g. used in the fee system), the ModuleRegistry, the FeatureRegistry, and the SecurityTokenRegistry.

    The PolymathRegistry is not proxied and cannot be upgraded. Ownership remains with Polymath.

    Main Functionality:

    • manage core components: set and change the address of core components.

  _(dot)

• SecurityToken

  _(dot)

  This is the central component of the system which implements ERC 1400, also known as the Security Token Standard, which is comprised of a group of standards that aim to create a representation of securities on the Ethereum blockchain. It is compatible with ERC 20 and ERC 777 and includes:

  • ERC 1410 - Partially Fungible Token Standard
  • ERC 1594 - Core Security Token Standard
  • ERC 1643 - Document Management Standard
  • ERC 1644 - Controller Token Operation Standard

  Security Tokens can have modules attached to them. Modules control the Security Tokens behavior and might have special capabilities or permissions in the system.

  To issue a new SecurityToken an issuer interfaces with the SecurityTokenRegistry to register a symbol and generate a new security token. The SecurityTokenRegistry defines which factory (STFactory) is used to deploy a new SecurityTokenProxy linked to a specific implementation of the SecurityToken. The SecurityTokenFactory by default adds a data store and default transfer manager module (unless no defaults are set) and sets the treasury wallet address for the security token. The issuer may then attach other modules to the security token.

  The SecurityToken is proxied and can be upgraded.
  Ownership of the SecurityTokenFactory (STFactory) and logic contracts (SecurityToken implementation) remains with Polymath. Ownership of the security token (SecurityTokenProxy) is finally transferred to the issuer.

  Main Functionality:

  • manage modules: add, remove, archive, unarchive, and upgrade modules
  • manage the security token: update token details and name, upgrade the token
  • manage security token transfers: transfer, freeze, and unfreeze transfers

  _(dot)

• Modules

  Modules control the security token's behavior and can have special capabilities or permissions in the system.

  The system comes with five types of modules:

  • TransferManager

    _(dot)

    A transfer manager controls the transfer of tokens. The following variants of transfer managers are provided by Polymath:

    • BlacklistTransferManager - prevent certain investors from transferring tokens during issuer-defined blackout periods
    • CountTransferManager - only allow up to a specified number of token holders
    • GeneralTransferManager - enforces whitelists, KYC, and other restrictions
    • LockupTransferManager - specify named lockup periods for certain investors
    • ManualApprovalTransferManager - transfers can only be made if pre-approved by token admin
    • PercentageTransferManager - limit the percentage of total tokens that can be owned by a single account
    • VolumeRestrictedTransferManager - limit the number of tokens that can be owned by a single account

    _(dot)

  • PermissionManager

    _(dot)

    The follow variants of permission managers are provided by Polymath:

    • GeneralPermissionManager - allows the issuer to add wallets as delegates and gives permissions to use restricted functions on other modules

  • CheckPoint

    _(dot)

    CheckPoint modules keep track of and make use of historical token balances.

    • DividendCheckpoint Modules

      Creates checkpoint when dividends are paid so token holders are paid proportionally according to the token balance they held at the time the dividend was created.

      The system provides two variants of DividendCheckpoint modules: An ERC20DividendCheckpoint and an EtherDividendCheckpoint.

    • VotingCheckpoint Modules

      Allow the token owner to create ballots for voting, where voting weights are determined by the token balances of each voter at the time the vote was created.

      • WeightedVoteCheckpoint

        Basic weighted vote checkpoint module.

      • PLCRVotingCheckpoint

        The partial-lock commit/reveal (PLCR) vote uses a commit/reveal scheme to keep votes private during the voting period. Votes are revealed when voting has completed.

    _(dot)

  • SecurityTokenOffering

    _(dot)

    SecurityTokenOffering modules allow security tokens to be issued in return for investment in various currencies (ETH, POLY and USD stable coin)

    • PresalesSTO - private presales

    • USDTieredSTO - price of token is specified in US dollars, with multiple price tiers

    • CappedSTO - sets a limit on the funding that is accepted and tokens that will be distributed

    _(dot)

  • Wallet

    _(dot)

    The system provides a VestingEscrowWallet by default that allows approved staff to create a token vesting schedule for employees/affiliates.

    _(dot)

• Oracles

  _(dot)

  Oracles are a trusted entity in the system. Polymath wraps the oracles in order to allow a manual price override or to freeze the price. For example, oracles are used with STO modules to convert USD prices into POLY or ether.

  _(dot)

1.4 Key Observations/Recommendations

Positive observations

• Best practices are followed where they make sense.
• Common design patterns are used where required instead of inventing new interfaces and designs.
• The token is following the ERC1400 Security Token Standard.
• An extensive test-suite with 1544 individual tests is provided with the framework.
• Arithmetic operations are performed using SafeMath.
• Smart contracts are split into multiple files, most of them being comprised of an interface and an implementation. Common functionality is provided by library contracts.
• Inline documentation for methods and classes is available and describes functionality and input parameters.
• The codebase did not yield compiler warnings that were in scope for this audit.
• The project provided easy to use scripts (npm/package.json) for flattening relevant contracts, executing the test-suite and generating coverage statistics.

Opportunities for improvement

• Test coverage is incomplete. Any contract system that is used on the main net should have as a minimum requirement a 100% test coverage especially with a risk profile of a Security Token Network. In particular it's useful to include negative test cases ensuring that undesirable changes are detected early and kept in-line with the specification. Security focused test cases verifying that permissions are set-up according to the security-specification and trust-model should be consistently implemented. Methods that require special permissions must be tightly covered. None of the contracts that comprise the contract system should be exempt from testing/coverage. Please not that having a 5% rate of uncovered statements can easily mean that a couple of hundred statements are uncovered for large projects, potentially including critical code paths as well.
• High Complexity. A multiple library/contract system is complex in nature. The Polymath system consists of a high number of individual contracts that need to be deployed and maintained. The inter-contract communication rate is high. The inheritance model of the system is complex.
• The projects npm run flatten-all script does not flatten all relevant modules.
• Improve the Documentation. Inconsistencies exist between the specification documents provided during the audit as well as the projects wiki.

It is recommended to fix all the issues listed in the following chapters, at the very least the ones with severity Critical, Major and Medium.

Individual issues are listed below, but it's important to note that the contract system is quite large, with complex proxying, upgradeability mechanisms, exponential combinations of modules, and insufficient testing. This means that there are likely vulnerabilities in the system that our team did not find. In addition to existing hidden vulnerabilities, new modules or upgrades to existing components could easily create new vulnerabilities. Fortunately, the administrative control retained by Polymath and token issuers will make it possible to recover from many types of vulnerabilities once discovered.

2 Issue Overview

The following table contains all the issues discovered during the audit. The issues are ordered based on their severity. More detailed description on the levels of severity can be found in the Severity Definitions Appendix. The table also contains the status of any discovered issue.

3 Issue Details

3.1 SecurityToken contract should always be initialized

Severity: Critical

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#672.

NB - we also fixed this in the SecurityTokenRegistry which had a similar issue.

Description

All security tokens are proxies that delegate all calls to SecurityToken implementation contract deployed by the Polymath. Since SecurityToken is a proxy itself, it's possible to delete the SecurityToken contract if it delegates calls to a malicious contract with selfdestruct. Because of that, it might be possible to use Create2 to replace the implementation with a malicious contract and steal tokens. Create2 attack can only be done by polymath, but anyone else can just delete the SecurityToken and break all the tokens irrevocably.

Remediation

Ensure initialization of the SecurityToken implementation contract with the safe getterDelegate. That prevents from creating an opportunity to destroy the contract or to use it maliciously. For example, it should be enough to add initialized = true; to SecurityToken constructor.

3.2 Unpredictable behavior due to front running or general bad timing

Severity: Critical

Status: Closed

Resolution: This issue has been addressed with the following comment:

PolymathRegistry / SecurityToken.updateFromRegistry: We have left this alone. In practice we don't expect issuers to need to call this (as all registries can be updated in place) and if they do, they would trigger it, not us.

New modules: We have left the logic as it is. If an issuer is concerned about this risk they can add the new module with _archived set to true which would add the module but not activate it. They can then check the details of the module added before activating it. They can also specify an upper bound on the module fee to avoid any front-running to change the fee.

New tokens: We have modified setProtocolFactory() to not allow changing the factory associated with an existing version. An issuer can now specify an exact version to deploy and we can't modify that version by front-running their transaction: PolymathNetwork/polymath-core#689

In general we will use the CD audit report to generate a transparency document specifying the areas that Polymath has control in, and the areas that we don't. The guiding principle is that, until the point that a token is created, Polymath has a good amount of control (e.g. through front-running an upgrade to an STFactory token) but once the token has been deployed we have no unilateral control (with certain documented exceptions, specifically that we can render the module registry inactive and have some initial access over the oracle pricing for STOs (see 3.3)).

Description

In a large number of cases, Polymath can update or upgrade things in the system without warning. This has the potential to violate a security goal of the system: once a security token has been created, Polymath shouldn't be able to interfere with that token's functioning.

Specifically, Polymath could use front running to make malicious changes just ahead of incoming transactions, or purely accidental negative effects could occur due to unfortunate timing of changes.

Some instances of this are more important than others, but in general token issuers should have assurances about the behavior of the action they're about to take.

Examples

The PolymathRegistry is used to store addresses for a wide variety of things, including oracles, the POLY token, security token registry, module registry, etc. By front running a call to SecurityToken.updateFromRegistry, Polymath could get a security token to use arbitrary addresses for the module registry, security token registry, and POLY token. In general, any call to PolymathRegistry.getAddress is vulnerable to front running.

When adding a new module to a token, the latest version is used from the module token registry. By front running a call to add a module to an existing security token, Polymath could add a new version of a module and cause the new code to be added to the existing security token. If the module is of the right type, e.g. one that allows minting and/or burning tokens, this could allow for direct theft of valuable assets.

Similarly, when deploying a new security token, the latest version is used from the security token registry. Someone creating a token thus has no way to be sure what version of the code they'll receive when they deploy a new token.

Remediation

The underlying issue is that users of the system can't be sure what the behavior of a function call will be, and this is because the behavior can change at any time.

There are two broad strategies for addressing this:

1. Let the user lock things down from changing. For example, allow them to specify what version of a module or token contract they expect, and if that version is no longer current by the time the transaction is processed, revert.
2. Give the user advance notice of changes with a time lock. For example, make all upgrades require two steps with a mandatory time window between them. The first step merely broadcasts to users that a particular change is coming, and the second step commits that change after a suitable waiting period.

Different aspects of the system may call for different specific remediations.

References

See also issue 3.3, which discusses the danger of oracle manipulation in particular.

3.3 Polymath can arbitrarily change prices during a USDTieredSTO token sale

Severity: Critical

Status: Closed

Resolution: This issue has been addressed with PolymathNetwork/polymath-core#691.

We have added a function modifyOracle that allows an issuer to take control of their own oracles if they wish to. The default is to use the Polymath "controlled" oracles. This follows the pattern of defaulting to have a certain amount of trust in Polymath but allowing an issuer to remove this if required.

Description

The USDTieredSTO token sale uses oracles to convert from USD to ETH or POLY. The oracle addresses come from the PolymathRegistry, which means that Polymath has the ability to swap out those oracles at any time. This enables Polymath to set an arbitrary price for themselves to participate in an STO, including a price of 0.

Remediation

We recommend putting time locks in place. It shouldn't be possible to replace the oracle implementations or to put them into manual override mode without announcing the change first and waiting a reasonable amount of time. This gives token issuers an opportunity to shut down any ongoing token sales before the oracle changes take effect.

3.4 No new ST can be created after implementation upgrade

Severity: Critical

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#694 (also see issue 3.12)

Description

Polymath should be able to upgrade SecurityToken implementation by creating a new security token contract. After that, a link to the new version should be set in STFactory contract by calling setLogicContract function.

code/contracts/tokens/STFactory.sol:L139-L148

function setLogicContract(string calldata _version, address _logicContract, bytes calldata _upgradeData) external onlyOwner {
    require(keccak256(abi.encodePacked(_version)) != keccak256(abi.encodePacked(logicContracts[latestUpgrade].version)), "Same version");
    require(_logicContract != logicContracts[latestUpgrade].logicContract, "Same version");
    require(_logicContract != address(0), "Invalid address");
    latestUpgrade++;
    logicContracts[latestUpgrade].version = _version;
    logicContracts[latestUpgrade].logicContract = _logicContract;
    logicContracts[latestUpgrade].upgradeData = _upgradeData;
    emit LogicContractSet(_version, _logicContract, _upgradeData);
}

The problem is that in this function initializationData is not set for the latest version of the logic contract. That means that all new security tokens will not have init data and they will not be initialized and most probably will revert on creation.

Remediation

Add logicContracts[latestUpgrade].initializationData initialisation in setLogicContract and check that initializationData length is at least 4 bytes.

3.5 ModuleRegistry - Custom modules can block their own removal

Severity: Major

Status: Closed

Resolution: closed with PolymathNetwork/polymath-core#698

Description

The module registry is controlled by the module registry owner. Both the registry owner and the module owner may remove modules from the registry. Modules are usually deployed by Polymath but the system also allows the use of custom modules. Custom modules can be enabled in the feature registry. This allows potentially untrusted entities to register modules while remaining in control of the module factory.

code/contracts/ModuleRegistry.sol:L207-L214

function removeModule(address _moduleFactory) external whenNotPausedOrOwner {
    uint256 moduleType = getUintValue(Encoder.getKey("registry", _moduleFactory));

    require(moduleType != 0, "Module factory should be registered");
    require(
        msg.sender == IOwnable(_moduleFactory).owner() || msg.sender == owner(),
        "msg.sender must be the Module Factory owner or registry curator"
    );

The order of the ownership checks in the current implementation give precedence to a potentially untrusted external call to the module factory to check if the sender is the module owner. Modules can therefore intentionally block their own removal by reverting on the external ownership call (IOwnable(_moduleFactory).owner()) . The check if the module registry owner is the sender will not be performed.

Remediation

Make sure the module registry owner can always remove modules from the registry. Avoid external calls to untrusted modules that might intentionally revert the execution. Give precedence to the trusted call and only after that call out to untrusted contracts.

Please note that unverifyModule() can also be blocked in the same way by a malicious module.

References

• also see issue 3.8

3.6 Transfer decisions by default should be consistent

Severity: Major

Status: Closed

Resolution: Logic has been modified so that transfers fail if there are no unarchived TMs. Documentation around the behaviour has been added: PolymathNetwork/polymath-core#693 https://github.com/PolymathNetwork/polymath-core/wiki/Transfer-manager-results

Description

code/contracts/tokens/SecurityToken.sol:L661-L696

function _executeTransfer(
    address _from,
    address _to,
    uint256 _value,
    bytes memory _data
)
    internal
    checkGranularity(_value)
    returns(bool)
{
    if (!transfersFrozen) {
        bool isInvalid;
        bool isValid;
        bool isForceValid;
        bool unarchived;
        address module;
        uint256 tmLength = modules[TRANSFER_KEY].length;
        for (uint256 i = 0; i < tmLength; i++) {
            module = modules[TRANSFER_KEY][i];
            if (!modulesToData[module].isArchived) {
                unarchived = true;
                ITransferManager.Result valid = ITransferManager(module).executeTransfer(_from, _to, _value, _data);
                if (valid == ITransferManager.Result.INVALID) {
                    isInvalid = true;
                } else if (valid == ITransferManager.Result.VALID) {
                    isValid = true;
                } else if (valid == ITransferManager.Result.FORCE_VALID) {
                    isForceValid = true;
                }
            }
        }
        // If no unarchived modules, return true by default
        return unarchived ? (isForceValid ? true : (isInvalid ? false : isValid)) : true;
    }
    return false;
}

If there are no active modules then _executeTransfer returns true by default. If there are active modules and all modules say NA transfer should have the same result, but it returns false.

Remediation

Make default decisions consistent. It looks like it makes more sense to make all transfers VALID by default.

3.7 ModuleRegistry - Custom module verification should be bounded to module version

Severity: Major

Status: Closed

Resolution: This issue has been addressed as part of issue 3.8. Module verification is not bound to version.

Description

If custom modules feature is enabled there is a mechanism of registering new custom modules by anyone. These modules can only be used by any other security token if polymath admins verify them. The problem is that verification is only bounded to module factory regardless of module version.

code/contracts/ModuleRegistry.sol:L245

function verifyModule(address _moduleFactory) external onlyOwner {

Once the factory is verified, the factory owner can upgrade a module to a malicious one.

Remediation

Add a module version to verification details and only allow a verified version to be used in security tokens. That would require polymath admins to review every new upgrade in a custom module. As an alternative, polymath admins can just make sure that they only verify module factories that are not upgradable.

3.8 ModuleRegistry - Risks of allowing custom modules in the system

Severity: Major

Status: Closed

Resolution: The functionality to register custom modules to the system remains available, increases complexity and extends the attack surface. A number of risks described with this issue have been addressed with PolymathNetwork/polymath-core#698.

• made useModule and registerModule non-reentrant as they make external calls to potentially untrusted contracts
• store factory owner on initial registration locally and reference this rather than calling back out to owner on the module factory. This means that a malicious factory cannot change the value of owner dynamically to e.g. allow security tokens to add it (by returning an owner equal the ST owner). The initial owner cannot be malicious as registerModule requires msg.sender to be the factory owner.
• fix issue where a malicious factory could return a type of 0, causing it to be un-removable
• modified getModulesByType to only return verified modules, so that a single malicious unverified module cannot grief this function.
• added getAllModulesByType can be used to return a raw list of all registered modules instead
• modify getFactoryDetails to also return the factory owner

This leaves the issue that someone could register ~8k new modules and cause loading moduleList to exceed the block gas limit. Whilst this is theoretically possible, in practice it would cost ~120 ETH and if it were to happen we could simply upgrade the ModuleRegistry to fix the problem. This should be a sufficient disincentive for anyone to attempt this griefing attack.

Description

The global setting customModulesAllowed is maintained in the FeatureRegistry and controls whether it is allowed to register custom modules in the ModuleRegistry for use with security tokens. By default only the ModuleRegistry owner can register modules. Setting customModulesAllowed opens up registration to anyone for custom modules. This has several security implications.

There are two scenarios in connection with custom modules:

1. A security token issuer would like to use their custom modules with their own security token (common use-case).

2. A module developer would like to make their module generally available to other users in the system (marketplace use-case). This requires the module to be verified by Polymath.

Registration

code/contracts/ModuleRegistry.sol:L171-L180

function registerModule(address _moduleFactory) external whenNotPausedOrOwner {
    if (IFeatureRegistry(getAddressValue(FEATURE_REGISTRY)).getFeatureStatus("customModulesAllowed")) {
        require(
            msg.sender == IOwnable(_moduleFactory).owner() || msg.sender == owner(),
            "msg.sender must be the Module Factory owner or registry curator"
        );
    } else {
        require(msg.sender == owner(), "Only owner allowed to register modules");
    }
    require(getUintValue(Encoder.getKey("registry", _moduleFactory)) == 0, "Module factory should not be pre-registered");

• Anyone (usually a security token owner) can register a moduleFactory by calling registerModule() provided the caller is the owner of the factory.
• After getting the factories moduleType, the factory is stored in the moduleList. The module is still unverified and can only be used by a security token owner that owns the factory. This gives anyone control over the size of the moduleList allowing someone to spam the list and potentially cause DoS conditions in other parts of the system that are iterating over all items in the moduleList (see subsection Listing modules)
• Note: the method is inefficient as it performs multiple external calls instead of re-using previously returned values (moduleFactory.types(), IOwnable(_moduleFactory).owner()).

Checking if a module can be used

code/contracts/ModuleRegistry.sol:L132-L140

function useModule(address _moduleFactory, bool _isUpgrade) external {
    if (IFeatureRegistry(getAddressValue(FEATURE_REGISTRY)).getFeatureStatus("customModulesAllowed")) {
        require(
            getBoolValue(Encoder.getKey("verified", _moduleFactory)) || IOwnable(_moduleFactory).owner() == IOwnable(msg.sender).owner(),
            "ModuleFactory must be verified or SecurityToken owner must be ModuleFactory owner"
        );
    } else {
        require(getBoolValue(Encoder.getKey("verified", _moduleFactory)), "ModuleFactory must be verified");
    }

Listing modules

code/contracts/ModuleRegistry.sol:L345-L371

function getModulesByTypeAndToken(uint8 _moduleType, address _securityToken) public view returns(address[] memory) {
    address[] memory _addressList = getArrayAddress(Encoder.getKey("moduleList", uint256(_moduleType)));
    uint256 _len = _addressList.length;
    bool _isCustomModuleAllowed = IFeatureRegistry(getAddressValue(FEATURE_REGISTRY)).getFeatureStatus(
        "customModulesAllowed"
    );
    uint256 counter = 0;
    for (uint256 i = 0; i < _len; i++) {
        if (_isCustomModuleAllowed) {
            if (IOwnable(_addressList[i]).owner() == IOwnable(_securityToken).owner() || getBoolValue(
                Encoder.getKey("verified", _addressList[i])
            )) if (isCompatibleModule(_addressList[i], _securityToken)) counter++;
        } else if (getBoolValue(Encoder.getKey("verified", _addressList[i]))) {
            if (isCompatibleModule(_addressList[i], _securityToken)) counter++;
        }
    }
    address[] memory _tempArray = new address[](counter);
    counter = 0;
    for (uint256 j = 0; j < _len; j++) {
        if (_isCustomModuleAllowed) {
            if (IOwnable(_addressList[j]).owner() == IOwnable(_securityToken).owner() || getBoolValue(
                Encoder.getKey("verified", _addressList[j])
            )) {
                if (isCompatibleModule(_addressList[j], _securityToken)) {
                    _tempArray[counter] = _addressList[j];
                    counter++;
                }

• Modules are organized in typed moduleLists and custom modules are not separated from built-in modules. This opens the possibility for a malicious actor to cause a DoS condition by filling the moduleList with dummy factories resulting in an out of gas condition while iterating the list.
• Even more straight forward a malicious module could just revert in the call to IOwnable(_addressList[i]).owner() causing the method to always fail.
• The check whether a module is verified should be performed first to allow a shortcut that is avoiding the external call checking whether the module factory is owned by the security token owner. Note that the external call may intentionally fail (issue 3.5).

code/contracts/ModuleRegistry.sol:L354-L355

if (IOwnable(_addressList[i]).owner() == IOwnable(_securityToken).owner() || getBoolValue(
    Encoder.getKey("verified", _addressList[i])

Remediation

There is considerable risk in allowing untrusted parties to provide and maintain custom modules in the system. Untrusted modules can be registered by anyone, are not separated from built-in modules and may therefore interfere with the system by causing Out-of-Gas DoS conditions, unexpected reverts or open up a potential for reentrancy attacks.

It is therefore recommended to assess whether this functionality is actually being used or required in the system. The general advice is to remove any functionality that is not actively used or still in development. To safely allow the use of custom modules it is recommended to rework the current code based on the changing trust assumptions.

• A custom module can only be used if the moduleFactory owner is the security token owner, or the moduleFactory is verified by Polymath. In order to get a module verified a manual step is required. In any case this step must include a thorough security inspection of the module to make sure that the moduleFactory and module implementations adhere to the interfaces provided by Polymath and to verify that the moduleFactory has no malicious intent, i.e. none of the module components can be updated after the verification has been performed.
• Trust assumptions are changing due to the external party maintaining full control of the module and its components (factory, implementation).
• A verification should always be bound to a specific version of a module (issue 3.7)
• Custom modules should be separated from built-in modules and the code should be refactored to make sure the existence of custom modules does not interfere with users relying on built-in modules (e.g. DoS/Out-of-Gas conditions).
• Always make sure that the registry owner is able to maintain and administer the registry and cannot be blocked by an external call (issue 3.5)
• Guard for potential reentrancy attacks.

3.9 _balanceOfByPartition function returns wrong value

Severity: Major

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#679.

// In UNLOCKED partition we are returning the minimum of all the unlocked balances // In locked partition we are returning the maximum of all the Locked balances

Description

code/contracts/tokens/SecurityToken.sol:L490

function _balanceOfByPartition(bytes32 _partition, address _tokenHolder, uint256 _additionalBalance) internal view returns(uint256 max) {

This function returns the maximum of the balances from all transfer managers. It should not be the case for UNLOCKED partition for example, because GTM may return all balance, while LTM can have half of the balance locked. In the result _balanceOfByPartition(UNLOCKED) will return full balance, but only half of it can actually be transferred.

Remediation

UNLOCKED partition balance should be the minimum of all balances from all transfer managers. There is an issue with that solution because some transfer managers do not implement this function properly and just return 0. That needs to be fixed by implementing getTokensByPartition function in every TM or adding a flag that shows whether this function is implemented.

LOCKED partition balances should remain the same (maximum).

3.10 _returnPartition function of SecurityToken always returns UNLOCKED partition

Severity: Major

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#680

Description

code/contracts/tokens/SecurityToken.sol:L538-L545

function _returnPartition(uint256 _beforeBalance, uint256 _afterBalance, uint256 _value) internal pure returns(bytes32 toPartition) {
    // return LOCKED only when the transaction `_value` should be equal to the change in the LOCKED partition
    // balance otherwise return UNLOCKED
    if (_afterBalance.sub(_beforeBalance) == _value)
        toPartition = LOCKED;
    // Returning the same partition UNLOCKED
    toPartition = UNLOCKED;
}

It's commented in the function that it should return LOCKED under some circumstances. Function _returnPartition always returns UNLOCKED partition. Additionally, the comment says that LOCKED should be returned when a change in the locked partition equals value but it actually checks the change in _partition which is usually UNLOCKED

Remediation

Review what actually should be returned here. If the initial intention is correct, actually check LOCKED partition balance change and return correct partition.

3.11 partitionsOf function always returns empty array

Severity: Major

Status: Closed

Resolution: addressed with PolymathNetwork/polymath-core#681 by hard-coding the partitions that are supported in 3.0.

Description

partitionsOf is a function inside STGetter contract that should return a list of the partitions for a particular token holder. _appendPartition function is used inside partitionsOf to append a new partition to the partitions array in case it's not already there. Instead, it does the opposite and only appends partitions that are already in the partitions array. Eventually, that results in the fact that no partitions are added and partitionsOf always returns an empty array.

Remediation

code/contracts/tokens/STGetter.sol:L279

if (duplicate) {

Change duplicate to !duplicate in if statement.

3.12 Security token upgrade info should be editable

Severity: Medium

Status: Closed

Resolution: This issue has been addressed with PolymathNetwork/polymath-core#694 (see issue 3.4) by adding functionality to edit upgrade info. Front-running opportunity remains.

Description

Polymath admins can upgrade security token implementation. In order to do so, they submit an address of a new token implementation and upgradeData for initialization. If they do it wrong, there is no way to edit that data and all tokens of lower versions will become non-upgradable.

Remediation

Allow editing security token upgrade information. It's important to avoid adding frontrunning opportunities while doing that.

3.13 VestingEscrowWallet - Integer Underflow and unchecked array access in pushAvailableTokensMulti()

Severity: Medium

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#673.

Description

pushAvailableTokensMulti() attempts to check if the argument _toIndex is within bounds of beneficiaries[]. However, the check already underflows at line 436 if beneficiaries is empty allowing the caller (withPerm(OPERATOR)) to provide any value for _toIndex that is smaller or equal to UINT256_MAX. The failed check may also lead to an invalid array access at line 438 causing an exception.

code/contracts/modules/Wallet/VestingEscrowWallet.sol:L435-L441

function pushAvailableTokensMulti(uint256 _fromIndex, uint256 _toIndex) public withPerm(OPERATOR) {
    require(_toIndex <= beneficiaries.length - 1, "Array out of bound");
    for (uint256 i = _fromIndex; i <= _toIndex; i++) {
        if (schedules[beneficiaries[i]].length !=0)
            pushAvailableTokens(beneficiaries[i]);
    }
}

Remediation

A check for _toIndex < beneficiaries.length is sufficient. Optionally also add a check to require _fromIndex <= toIndex to avoid that the method silently returns without performing an action on invalid input.

References

• see VestingEscrowWallet.sol.mythril.out

3.14 SecurityTokenRegistry does not inherit from ISecurityTokenRegistry

Severity: Medium

Status: Closed

Resolution: This issue has been addressed with PolymathNetwork/polymath-core#682 updating the mismatching interface but not inheriting from it (due to some functions living in the proxied STRGetter). Recommendation: Add dev-note to SecurityTokenRegistry reminding dev to update interface as it is not automatically enforced.

We did not inherit ISTR from STR as this doesn't quite work as some of the functions in ISTR live in the STRGetter (due to contract size limitations). We did review and align the functions between these contracts though in: PolymathNetwork/polymath-core#682.

Description

SecurityTokenRegistry.sol imports ./interfaces/ISecurityTokenRegistry.sol but it is not used in the code.

code/contracts/SecurityTokenRegistry.sol:L7

import "./interfaces/ISecurityTokenRegistry.sol";

Remediation

SecurityTokenRegistry should inherit from ISecurityTokenRegistry to make sure it is an implementation compliant to the interface.

3.15 Different implementations for the same modifier whenNotPausedOrOwner

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#674.

Description

ModuleRegistry and SecurityTokenRegistry provide different implementations for the same modifier.

code/contracts/ModuleRegistry.sol:L82-L88

modifier whenNotPausedOrOwner() {
    if (msg.sender == owner()) _;
    else {
        require(!isPaused(), "Already paused");
        _;
    }
}

code/contracts/SecurityTokenRegistry.sol:L168-L176

modifier whenNotPausedOrOwner() {
    _whenNotPausedOrOwner();
    _;
}

function _whenNotPausedOrOwner() internal view {
    if (msg.sender != owner())
        require(!isPaused(), "Paused");
}

Remediation

Stick to one variant of the implementation to improve code maintainability.

3.16 SecurityToken - security token name change may cause inconsistency

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#706.

We modified the code so that token name is only stored on the ST itself, and the STR retrieves the details from the ST directly.

To retain backwards compatibility, if there is a name stored in the STR it is returned.

Description

When generating registering a ticker and generating a security token a name for the security token is specified. The token name is stored in the security token registry while registering the ticker name (_storeTickerDetails(), registeredTickers_tokenName<TokenName>). However, an issuer can unilaterally change the name of the token stored in the security token (see related issue issue 3.18). This update is not reflected in the security token registry causing an inconsistency with the security token.

code/contracts/SecurityTokenRegistry.sol:L462-L482

function _storeTickerDetails(
    string memory _ticker,
    address _owner,
    uint256 _registrationDate,
    uint256 _expiryDate,
    string memory _tokenName,
    bool _status
)
    internal
{
    bytes32 key = Encoder.getKey("registeredTickers_owner", _ticker);
    set(key, _owner);
    key = Encoder.getKey("registeredTickers_registrationDate", _ticker);
    set(key, _registrationDate);
    key = Encoder.getKey("registeredTickers_expiryDate", _ticker);
    set(key, _expiryDate);
    key = Encoder.getKey("registeredTickers_tokenName", _ticker);
    set(key, _tokenName);
    key = Encoder.getKey("registeredTickers_status", _ticker);
    set(key, _status);
}

Remediation

Indicate that the security token name stored in the registry is the initial token name or consider to disallow security token owners to change the name of their token.

3.17 Where possible, a specific contract type should be used rather than address

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#683.

Description

Rather than storing addresses and then casting to the known contract type, it's better to use the best type available so the compiler can check for type safety.

Examples

ModuleStorage.securityToken is of type address, but it could be type ISecurityToken instead. Not only would this give a little more type safety when deploying new modules, but it would avoid repeated casts throughout the codebase of the form ISecurityToken(securityToken).

Remediation

Where possible, use more specific types instead of address. This goes for parameter types as well as state variable types.

3.18 SecurityToken - Missing Input Validation changeName

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#695.

Description

The security token name is initially set and validated when calling SecurityTokenRegistry.generateSecurityToken(). After the new security token proxy has been deployed ownership is transferred to the issuer. The issuer can then call changeName on the security token to set a new name without enforcing any restrictions.

SecurityTokenRegistry initial deployment and input validation

code/contracts/SecurityTokenRegistry.sol:L564-L575

function generateNewSecurityToken(
    string memory _name,
    string memory _ticker,
    string memory _tokenDetails,
    bool _divisible,
    address _treasuryWallet,
    uint256 _protocolVersion
)
    public
    whenNotPausedOrOwner
{
    require(bytes(_name).length > 0 && bytes(_ticker).length > 0, "Bad ticker");

SecurityToken - no input validation

code/contracts/tokens/SecurityToken.sol:L361-L365

function changeName(string calldata _name) external {
    _onlyOwner();
    emit UpdateTokenName(name, _name);
    name = _name;
}

Remediation

Make sure to enforce consistent input validation while deploying a new security token, but also when allowing an issuer to change certain variables.

3.19 deleteDelegate should be implemented without array iteration in permission manager

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#696.

The approach taken here was to leave the array iteration, but modify deleteDelegate so that it can be used to prune the list of delegates without iterating through the entire list.

Since delegates are set by the issuer, and a reasonable number of delegates is 1 - 5, this seems a reasonable fix to us.

Description

deleteDelegate function of GeneralPermissionManager contract iterates over all delegates to find the right one. It might become impossible to delete a delegate because of the array size (though it's unlikely, it still becomes more expensive). There should be a more efficient way to remove a delegate.

Remediation

Store indexes of delegates in the array or have a view function to determine the array index and pass that index to the deleteDelegate.

3.20 Improve Code Reusability - Use const variables instead of literals for EIP-1066 status codes.

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#690. Severity rating has been downgraded to Minor.

Description

EIP-1066 Status code literals are used throughout the codebase which makes it hard to maintain and understand the codebase.

code/contracts/libraries/TokenLib.sol:L458-L483

function canTransfer(
    bool success,
    bytes32 appCode,
    address to,
    uint256 value,
    uint256 balanceOfFrom
)
    public
    pure
    returns (bool, byte, bytes32)
{
    if (!success)
        return (false, 0x50, appCode);

    if (balanceOfFrom < value)
        return (false, 0x52, bytes32(0));

    if (to == address(0))
        return (false, 0x57, bytes32(0));

    // Balance overflow can never happen due to totalsupply being a uint256 as well
    // else if (!KindMath.checkAdd(balanceOf(_to), _value))
    //     return (false, 0x50, bytes32(0));

    return (true, 0x51, bytes32(0));
}

Remediation

Consider creating a Library/SubContract that exports const EIP-1066 Status Codes with descriptive variable names for use in the codebase.

References

• http://eips.ethereum.org/EIPS/eip-1066

3.21 TokenLib - Unused Import KindMath

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#684.

Description

TokenLib imports ./KindMath.sol but it is never used.

code/contracts/libraries/TokenLib.sol:L10

import "./KindMath.sol";

Remediation

Remove unused imports.

3.22 Module - authentication modifier onlyFactoryOwner and onlyFactoryOrOwner are never used

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#685.

Description

The modifiers onlyFactoryOwner and onlyFactoryOrOwner are defined in Module but never used. This can indicate a lack of authentication control for methods exposed by any of the modules inheriting from Module but can also be a preparation for future modules. The modifiers check if msg.sender is the modules factory owner or msg.sender is the factory calling the method. The modules factory is set in the constructor of ModuleStorage when initializing the module.

code/contracts/modules/Module.sol:L48-L56

modifier onlyFactoryOwner() {
    require(msg.sender == Ownable(factory).owner(), "Sender is not factory owner");
    _;
}

modifier onlyFactoryOrOwner() {
    require((msg.sender == Ownable(securityToken).owner()) || (msg.sender == factory), "Sender is not factory or owner");
    _;
}

code/contracts/storage/modules/ModuleStorage.sol:L28

factory = msg.sender;

Remediation

A general recommendation is to remove unused functionality and re-introduce it when there is actually an implementation that is using it. This makes it easier to understand what the code is actually doing and does not pose a risk of someone assuming certain security checks are in place. Verify all exposed interfaces of contracts inheriting from Module for missing authentication. Remove the modifier if the TokenFactory does not call to any of the modules and no special authentication is required.

3.23 SecurityToken - authentication modifier onlyTokenFactory is never used

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#686.

Description

The modifier onlyTokenFactory is defined in SecurityToken but never used. This can indicate a lack of authentication control for methods exposed by SecurityToken. The modifier checks if msg.sender is the tokenFactory that has been set while initializing the contract.

code/contracts/tokens/SecurityToken.sol:L169-L172

modifier onlyTokenFactory() {
    require(msg.sender == tokenFactory);
    _;
}

Remediation

A general recommendation is to remove unused functionality and re-introduce it when needed. This makes it easier to understand what the code is actually doing and does not pose a risk of someone assuming certain security checks are in place. Verify exposed interfaces of SecurityToken for missing authentication. Remove the modifier if the TokenFactory does not call SecurityToken and no special authentication is required.

3.24 SecurityToken/STGetter (ERC1643) getAllDocuments - can be cause gas or memory issues if used

Severity: Minor

Status: Closed

Resolution: This issue has been addressed with the following statement:

In general, for getter functions which are only expected to be called off-chain, and can therefore have a very high gas limit (beyond the block gas limit) we are comfortable having functions which potentially have high gas usage. We have this pattern in getters (intended only for a dApp) throughout our code.

Description

The function getAllDocuments() can be blocked if too many documents are added. Contracts interacting with it might run out of gas. This can also create problems with other code that relies or tries to read the result value of getAllDocuments().

Because the output is unlimited, this can cause out of gas, memory or execution problems.

This problem can be avoided if there is no code that interacts with this function or the number of documents will always be a low number.

Examples

Calling the getAllDocuments() function has this gas impact with the specified number of documents:

• 200 documents will use ~100k gas;
• 1000 documents will use ~370k gas;
• 1700 documents will use ~650k gas;
• 5000 documents will use ~2 million gas.

This contract was used to generate the gas costs above:

contract Reader {
    ERC1643 public d;
    constructor(ERC1643 _addr) public {
        d = _addr;
    }
    
    uint256 public count;
    
    function readDocs() public returns (uint256) {
        count = d.getAllDocuments().length;
        return count;
    }
}

Remediation

Because this is part of the standard implementation and the specs, there is no code remediation possible. The advice is to avoid calling the function and only use getDocument().

References

• Geth provides MaxUint64 / 2 gas when calling view, or constant functions but this was not always the case:

internal/ethapi/api.go:L740

gas := uint64(math.MaxUint64 / 2)

• https://ethereum.stackexchange.com/a/9872

• https://ethereum.stackexchange.com/a/44778/6253

Ganache and Parity have different defaults. Other clients will implement their own defaults.

3.25 PolyToken - events are redefined in the implementation

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#692 by removing PolyToken from the repository.

Description

The PolyToken implementation extends the IPoly interface which defines the ERC20 events (Transfer, Approval) that can be emitted in the contract. Because the events are already defined in the interface, they do not need to be defined again in the implementation.

Defining the events again can lead to mistakes, in case the developer defines a similar but not quite exact event, or confusion, because somebody that reads the contract now needs to check if the specified event in the implementation matches the one defined in the interface.

Having the events defined only in one place reduces confusion and possible mistakes, while increasing code maintainability.

code/contracts/interfaces/IPoly.sol:L17-L18

event Transfer(address indexed from, address indexed to, uint256 value);
event Approval(address indexed owner, address indexed spender, uint256 value);

code/contracts/helpers/PolyToken.sol:L80-L81

event Transfer(address indexed from, address indexed to, uint256 value);
event Approval(address indexed owner, address indexed spender, uint256 value);

Examples

The code when emitting the Boom() event in each case is very similar but the emitted event might not be the expected one.

pragma solidity >0.5.0;

interface MyInterface {
    function explode(bytes calldata _data) external;
    function implode(uint256 _data) external;
    
    event Boom(bytes _data);
}

contract MyImplementation is MyInterface {
    // We define an event with the same name as the one in the interface
    // but a different argument type
    event Boom(uint256 _data); 
    
    /*
    This function emits the MyInterface.Boom(bytes _data) event:
    [
    	{
    		"from": "0xec5bee2dbb67da8757091ad3d9526ba3ed2e2137",
    		"topic": "0xf0525cb4f38ada4b00a047310c294a112a153e48e11a55b7f4b2fe1024405383",
    		"event": "Boom",
    		"args": {
    			"0": "0x1234",
    			"_data": "0x1234",
    			"length": 1
    		}
    	}
    ]
    */
    function explode(bytes memory _data) public {
        emit Boom(_data);
    }
    
    
    /*
    This function emits the MyImplementation.Boom(uint256 _data) event:
    [
    	{
    		"from": "0xec5bee2dbb67da8757091ad3d9526ba3ed2e2137",
    		"topic": "0x1167d8fb28af59d807f1ba2dd442f6a78306131cc03e11970dd6e66c8adc9f57",
    		"event": "Boom",
    		"args": {
    			"0": "4660",
    			"_data": "4660",
    			"length": 1
    		}
    	}
    ]
    */
    function implode(uint256 _data) public {
        emit Boom(_data);        
    }
}

Remediation

Remove the events which are already defined in the interface.

3.26 Use latest stable version of Solidity

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#675.

Description

Solidity is very active in pushing out new versions. Make sure to include the latest stable version that is compatible with your code. The codebase is currently using solidity v0.5.0.

code/flat/FeatureRegistry.sol:L1

pragma solidity ^0.5.0;

Remediation

At the moment of writing, the latest stable version is 0.5.8.

It is a good practice to pin to a specific version. Having a floating version can open your application to newer, possibly not well tested, versions (SWC-103).

// Fixed version
pragma solidity 0.5.8; 

// As opposed to a floating pragma
pragma solidity ^0.5.8;

References

https://github.com/ethereum/solidity/releases

3.27 OwnedProxy should be removed

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#701.

Description

The OwnedProxy contract is only used in the DataStoreProxy, and Proxy would be a better choice there. In general, there's no compelling use case for this contract, as ownership isn't being used for anything.

Remediation

Have DataStoreProxy inherit from Proxy instead and then delete the dead OwnedProxy code.

3.28 Module.takeUsageFee() allows admins to drain a module's approved POLY tokens

Severity: Minor

Status: Closed

Resolution: This issue has been closed by removing the functionality with PolymathNetwork/polymath-core#700.

Description

There are no limits to when and how often Module.takeUsageFee() can be called, and failure to collect has no on-chain consequences. It currently allows any admin to drain a module of approved POLY tokens. It, in fact, prevents modules from properly implementing POLY-based usage fees unless they take care to override this function.

Remediation

Remove Module.takeUsageFee(). If future modules want to collect a fee, they can implement a (safe) version of this function when it is needed.

Optionally, also remove ModuleFactory.usageCost and ModuleFactory.usageCostInPoly(), which will be completely unused if takeUsageFee() is removed.

3.29 Redundant pause and unpause functions in TransferManager

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#676

Description

code/contracts/modules/TransferManager/TransferManager.sol:L19-L27

function unpause() public {
    _onlySecurityTokenOwner();
    super._unpause();
}

function pause() public {
    _onlySecurityTokenOwner();
    super._pause();
}

pause and unpause functions implementations duplicate checks from Module contract that TransferManager inherits.

Remediation

Remove pause and unpause functions from TransferManager contract.

3.30 Modules shouldn't be casted to IBoot

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#687.

Description

code/contracts/modules/ModuleFactory.sol:L222

bytes4 initFunction = IBoot(_module).getInitFunction();

Modules do not explicitly inherit IBoot even though IModule and IBoot have the same function getInitFunction.

Remediation

Either cast _module to IModule or make IModule inherit IBoot.

3.31 public functions in TokenLib could be external instead

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#678.

Description

Most, if not all, public functions in TokenLib could be made external instead. This would save on gas at both deployment time and runtime. It would also make the code easier to read/audit by eliminating the possibility of internal callers.

Note that these functions could instead be made internal, but this would cause them to be inlined into the calling contract, and we believe it was an intentional decision to use TokenLib as a way to reduce the size of the calling contract.

Remediation

Mark the functions as external instead of public wherever possible.

3.32 EtherDividendCheckpoint and ERC20DividendCheckpoint division by zero

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#697.

Description

EtherDividendCheckpoint._createDividendWithCheckpointAndExclusions and ERC20DividendCheckpoint._createDividendWithCheckpointAndExclusions reject dividends when the currentSupply is zero, presumably to prevent future division by zero errors when calculating dividend payouts. Code for EtherDividendCheckpoint:

code/contracts/modules/Checkpoint/Dividend/Ether/EtherDividendCheckpoint.sol:L134

require(currentSupply > 0, "Invalid supply");

However, the actual totalSupply for the dividend is calculated by subtracting balances of excluded addresses from currentSupply. This means that a zero is possible:

code/contracts/modules/Checkpoint/Dividend/Ether/EtherDividendCheckpoint.sol:L152-L158

for (uint256 j = 0; j < _excluded.length; j++) {
    require(_excluded[j] != address(0), "Invalid address");
    require(!dividends[dividendIndex].dividendExcluded[_excluded[j]], "duped exclude address");
    excludedSupply = excludedSupply.add(ISecurityToken(securityToken).balanceOfAt(_excluded[j], _checkpointId));
    dividends[dividendIndex].dividendExcluded[_excluded[j]] = true;
}
dividends[dividendIndex].totalSupply = currentSupply.sub(excludedSupply);

(The same issue is present in ERC20DividendCheckpoint.)

The impact of this bug is minor because no payouts will be possible on a dividend with no eligible balances.

Remediation

The best fix is probably to move the require statement down below the totalSupply calculation and base it on that value:

require(dividends[dividendIndex].totalSupply > 0, "Invalid total supply");

3.33 SecurityToken / STGetter storage layout is hard to maintain

Severity: Minor

Status: Closed

Resolution: This issue has been addressed with PolymathNetwork/polymath-core#704.

We added a script to our deployment pipeline to check that memory is laid out correctly.

Description

SecurityToken contract delegates to STGetter as a workaround for contract deployment size limitations. As is the case for any delegation between contracts, the two contracts need to maintain identical storage layouts.

Unlike in other proxies throughout the codebase, SecurityToken and STGetter do not inherit from the same base storage contracts to enforce the storage layout. Rather, STGetter uses the OZStorage contract. The "OZ" presumably stands for "OpenZeppelin" because the contract mimics the storage layout of the ERC20 and ReentrancyGuard contracts from OpenZeppelin.

Although the current code looks correct, maintaining the relationship between SecurityToken and STGetter's contract storage is both difficult and critical from a security perspective.

Remediation

1. Replace OZStorage with ERC20 and ReentrancyGuard directly in STGetter inheritance. This has the advantage of making the code more transparent and eliminating OZStorage altogether. It will make STGetter more expensive to deploy, but this is a one-time cost.
2. At a minimum, add comments to OZStorage, SecurityToken, STGetter that explains the issue, so future code maintainers remember to ensure consistent storage layout.

3.34 LockUpTransferManager._removeLockUpFromUser checks for an impossible condition

Severity: Minor

Status: Closed

Resolution: This issue has been closed with PolymathNetwork/polymath-core#688.

Description

The following lines check whether an index is equal to an array length, but such a thing cannot occur except in error cases that would cause the transaction to revert anyway. Presumably there's a missing - 1 in both places:

code/contracts/modules/TransferManager/LTM/LockUpTransferManager.sol:L553

if ( _lockupIndex != _len) {

code/contracts/modules/TransferManager/LTM/LockUpTransferManager.sol:L563

if ( _userIndex != _len) {

In both cases, the erroneous check is harmless because it was just attempting to save gas anyway by avoiding a no-op swap.

Remediation

Check against _len - 1 instead.

4 Threat Model

The creation of a threat model is beneficial when building smart contract systems as it helps to understand the potential security threats, assess risk, and identify appropriate mitigation strategies. This is especially useful during the design and development of a contract system as it allows to create a more resilient design which is more difficult to change post-development.

A threat model was created during the audit process in order to analyze the attack surface of the contract system and to focus review and testing efforts on key areas that a malicious actor would likely also attack. It consists of two parts: a high-level design diagram and component description that helps to understand the attack surface and a list of security stories describing the contract system.

4.1 Overview

Actors

Polymath

• is in control of the account that deploys the polymath contract system.
• operates the polymath contract system.
• owns, manages and has full permissions to the polymath network registries (Feature-, Polymath-, Module-, SecuritytokenRegistry) and their proxies.
• can alter the addresses of core system contracts (polymath registry).
• can enable/disable system-wide features (feature registry).
• can globally allow security token issuers to use custom modules with their security tokens (feature registry).
• provides modules, module factories and deploys module proxies that are available to security tokens (module registry).
• controls which modules are generally available to issuers in the system.
• can make custom modules generally available to other issuers in the system (verifyModule).
  • it might not be possible for issuers to distinguish a custom module from one that is maintained by polymath.
• does not necessarily own custom modules (can be created by anyone; usually owned by issuer to be available to security tokens).
• owns the Security Token Factory that is used to deploy security tokens.
• deploys SecurityTokenProxy and transfers ownership to issuer.
  • cannot interfere with security tokens once they are generated and ownership has been transferred.
  • trust assumption: issuer remains in full control of their security token.
  • polymath still remains some level of control via referenced contracts (e.g. oracles, registries, factories, ...).
• controls the issuance of new security tokens and can stop people from generating new tokens.
• remains control over some parts of the contracts even when they are paused.
• deploys and remains in full control of oracle interface contracts.
  • can manually override and freeze responses provided by the external oracle provider.
  • sanity checks oracle responses.
• manages the fee system (module fees; potentially usage fees).
  • owns the poly token.

Oracles

• controls conversion rates.
• are a trusted entity in the system.
  • can be overridden by Polymath.
  • are controlled by Polymath oracle interface contracts.

Issuer

• can register new token symbols/tickers.
• can generate a new security token and finally owns it.
• manages the security token: minting, add/remove modules.
• can delegate permissions about certain aspects of the security token to other entities (admin/operate permissions).
• can add custom modules if allowed by the feature-registry.
• can create/deploy/maintain and use custom modules (if allowed by feature registry).
• pays fees.

Investor

The investors are the external users that initially have to be verified off-chain. Depending on the configuration of the security token they are whitelisted on-chain to be able to transact the tokens. They do not have any special permissions and by design they only interact with the tokens and the interest distribution.

• can own sec tokens shares.
• can transfer tokens.
• does not have any major permissions.
• can vote in platform.
• can call for dividend payments.

Module Operator

This role is assigned separately for each module by the SecurityToken owner in permission manager. Operator from one module does not have any rights in any other module. The Operator role protects access to critical functionality.

Module Admin

This role is assigned separately for each module by the SecurityToken owner in permission manager. Admin from one module does not have any rights in any other module. The Admin role protects access to functionality that is even more critical than what is protected by the Operator role.

Other ethereum accounts

• can deploy and register custom modules if allowed by global feature registry. Modules can only be used if they are verified by polymath.

ContractOwners

The following contracts are ownable:

• Registries / RegistryProxies - owned by Polymath
• SecurityTokenFactory - owned by Polymath
• SecurityTokenProxy - owned by issuer
• Modules/ModuleProxies/ModuleFactories - owned by Polymath
• Custom Modules/ModuleProxies/ModuleFactories - owned by issuer or anyone if allowed by global feature registry.

Assets

Assets must be protected, as potential threats could result in considerable loss for the actors, can erode the system's trust or have legal repercussions. The following assets were identified:

• Private keys: a fair number of contracts are deployed in the system. different private keys have special control over features in the contracts.
• Deployed contracts: the instances of the on chain deployed contracts are considered an asset.
• Documents: documents with information handled by the ERC 1643 implementation; the documents are not added on chain, however metadata (Name, URI, DocumentHash) is stored in the contracts.
• Registries: Owned by polymath, the registries are the backbone of the system. They provide access to the core functionality and deployed contracts.
• SecurityTokens: Owned by the issuer, the security tokens themselves with their security token configuration are considered an asset.
• Modules: Owned by polymath - or if allowed by the global feature registry, owned by any other entity in the system - individual modules and their functionality are considered an asset.
• Balances: ETH and ERC20 tokens balances managed by components of the system.
• Partitions/Shares/Stake: represent different slates of investor balances with specific sets of rules.

Detail

Overview

The following diagram gives a high-level overview of the system, including its core components and an initial list of actors, public interfaces and external calls as well as notes taken during the audits exploration phase.

Process: Security Token & Modules

The following diagram gives a high-level overview of the security token generation process, the patterns used and the relationship to the module life-cycle.

4.2 Security Stories

This section provides an excerpt of various security stories the team explored during the audit. Security stories aim to bring transparency about security relevant characteristics of the system, help to understand trust assumptions and describe potential high-level threats to the system. The goal is to spark security discussions, document them as part of a continuous process and even use them as input for internal SDL security practices. Relevant issues derived from the stories are listed in the Issue Details section.

Polymath

• Polymath is in control of registries forming the backbone of the system, may modify the location of other system components (PolymathRegistry), enable and disable features (FeatureRegistry), register and verify modules that are available to security tokens (ModuleRegistry) and may front-run users by changing attributes/parts of the system.
• Polymath is in control of the security token generation, may disable token generation, delist and modify tokens reference in the registry.
• Polymath is in control of the security token registration, may disable ticker registration, modify/delete registered tickers.
• Polymath does not retain direct control of deployed security tokens once ownership was transferred to the issuer.
• Polymath remains indirect control of the security token via system components referenced in the security token (oracle, registries, ...) and may therefore indirectly interfere with the security token.
• Polymath can delist a security token and immediately list a new one with the same ticker. This might be a problem if done, for example, immediately before a hyped token sale if buyers are going to find the token via Polymath's registry rather than directly by address. The buyers could then end up buying the wrong token.
• Polymath is in control of Oracles and may freeze or manually override prices.
• Polymath is in control of the fee system and may arbitrarily change fees (front-run).

Issuer

• An issuer can register arbitrary and potentially insulting ticker symbols which may cause manual effort to be corrected.
• An issuer can create and add custom modules to security tokens that are unverified by Polymath if custom modules are generally allowed in the system, which may undermine trust assumptions.

Oracle

• An oracle can try to provide untruthful responses to manipulate prices in the system. Oracle responses are controlled by contracts owned by Polymath.

Other participants

• Other participants can observe ticker/token registrations.
• Other participants can observe and collect document information linked to security tokens.
• Other participants can attempt to register custom modules containing dangerous functionality and try to get them verified by Polymath to make them available for other participants if custom modules are allowed by the system.
• Other participants can attempt to spam the custom module registration to cause DoS scenarios (out of gas) for other participants if custom modules are allowed by the system.
• Other participants may be able to claim ownership of contracts that were left uninitialized by accident (e.g. SecurityToken, SecurityTokenProxy, ...)
• Other participants can attempt to perform ticker squatting, trying to register popular ticker names before others do, sell them or create manual effort due to filed disputes.
• Other participants can attempt to front-run ticker registrations which can result in a frustrating experience for potential issuers or result in manual effort due to filed disputes.
• Other participants may attempt to get control of system components by getting hold of other participants' private keys (extortion, social engineering, and traditional information security threats).

5 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.

5.1 Mythril

Mythril is a security analysis tool for Ethereum smart contracts. It uses concolic analysis to detect various types of issues. The tool was used for automated vulnerability discovery for all audited contracts and libraries. More details on Mythril's current vulnerability coverage can be found here.

The raw output of the Mythril vulnerability scan can be found here.

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.

5.2 Harvey

Harvey is a greybox-fuzzer that uses light-weight program instrumentation to record the execution path for each tested input and compute its path identifier. The approach was developed by Valentin Wüstholz and Maria Christakis and published in their 2018 paper.

5.3 Ethlint

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

The raw output of the Ethlint vulnerability scan can be found here. The raw output of the Solhint scan can be found here.

5.4 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.

1. A complete list of functions with their visibility and modifiers can be found here.
2. A visualization of the system's function call graph can be found here (dot).
3. A visualization of the system's inheritance graph can be found here (dot).

6 Test Coverage Measurement

Code coverage metrics indicate the amount of lines/statements/branches that are covered by the test-suite. It's important to note that "100% test coverage" is not a silver bullet. Our review also included an inspection of the test suite to ensure that testing included important edge cases. Be aware that code coverage does not provide information about the individual test-cases quality.

A fork of the Solidity-Coverage tool was used to measure the portion of the code base exercised by the test suite, and identify areas with little or no coverage. Specific sections of the code where necessary test coverage is missing are included in the Issue Details section.

The project is using the automated testing framework provided by Truffle. The test-suite is evaluating 1556 individual tests and the test-suite passed without errors. The corresponding console output can be found here.

A code coverage report was generated and is provided along other tool output. The test coverage results can be viewed by opening the index.html file from the coverage report directory in a browser. Please find a summary of the coverage results below.

6.1 Test Coverage observations

• 95.12% of Statements, 83.35% of Branches, 94.62% of Functions and 95.18% Lines are covered by test-cases
  • It is important to provide an extensive test-suite that covers all statements, functions and paths of the contract system. Any contract system that is used on the main net should have as a minimum requirement a 100% test coverage especially. Please note that a seemingly high coverage rate of 95% means that 5% of Statements are uncovered. A few percent can easily translate to a couple of hundred uncovered statements for large codebases.
• The Branch coverage of 83.35% (257 uncovered branches) is low.
• Negative testing of require() assertions or edge-cases is often missing contributing to the low branch coverage. It is suggested to add positive and negative tests especially for important language constructs like require that often perform input validation.
• Some functions are completely uncovered or exempt from testing (see below).

No coverage stats available

The following contracts are not part of the coverage statistics collection:

• contracts/storage - contains storage layouts for other modules but also some logic (initialization) that should be tested
• contracts/oracles - contains oracle interface contracts written and maintained by polymath that should be tested.
• contracts/modules/experimental - new experimental modules that should be tested.
• contracts/external - contains externally provided OraclizeI/IMedianizer contracts that should be mocked for testing.
• contracts/helpers/polytoken - is the ERC20 POLY token that should be tested but is out of scope for this audit.
• contracts/libraries/KindMath, BokkyPooBahsDateTimeLibrary - are 3rd party libraries with unclear test/coverage status.

Contract Methods lacking coverage

• STRGetter
  • uncovered: getExpiryLimit()
  • partially: _ownerInTicker()
• SecurityTokenRegistry
  • uncovered: updateFromRegistry(), getSecurityTokenLaunchFee(), getTickerRegistrationFee(), setGetterRegistry(), removeProtocolFactory()
  • partially: generateNewSecurityToken(), transferTickerOwnership()
• Encoder
  • uncovered: getKey()
• TokenLib
  • uncovered: hash(), extractSigner()
• Module
  • uncovered: modifier(onlyFactoryOwner,onlyFactoryOrOwner)
• *ModuleFactory
  • uncovered: changeTags()
  • partially: changeSTVersionBounds(), usageCostInPoly()
• DividendCheckpoint
  • uncovered: isClaimed()
• TransferManager
  • uncovered: getTokensByPartition(), getPartitions()
• BlacklistTransferManager
  • uncovered: getAllBlacklists()
• CountTransferManager
  • uncovered:verifyTransfer()
• GeneralTransferManager
  • partially: _modifyInvestorFlag()
• LockupTransferManager
  • uncovered:verifyTransfer(), addLockUpByNameMulti(), removeLockUpFromUser(), removeLockupTypeMulti(), modifyLockUpTypeMulti(), getListOfAddresses(), getLockupsNamesToUser()
  • partially: _modifyLockUpType(), _addNewLockUpType(), getTokensByPartition()
• ManualApprovalTransferManager
  • uncovered: modifyManualApprovalMulti()
  • partially: _modifyManualApproval()
• PercentageTransferManager
  • uncovered:verifyTransfer()
• VolumeRestrictionTM
  • uncovered:addIndividualDailyRestrictionMulti(), removeIndividualDailyRestrictionMulti(),removeDefaultDailyRestriction(), modifyIndividualDailyRestrictionMulti(), modifyDefaultRestriction(), modifyDefaultDailyRestriction(), getPermissions()
  • partially: verifyTransfer(), addIndividualDailyRestriction()
• VestingEscrowWallet
  • uncovered:getTreasuryWallet()
• OwnedProxy
  • uncovered: modifier(ifOwner), proxyOwner(), implementation(), transferProxyOwnership()
• OZStorage
  • uncovered: totalSupply()
• STFactory
  • uncovered: updateDefaultTransferManager(), updateDefaultDataStore()
• STGetter
  • uncovered: partitionsOf(), _appendPartition()
• SecurityToken
  • uncovered: modifier(onlyTokenFactory), canTransferFrom(), disableController(), freezeIssuance()
  • partially: _canTransfer(), _executeTransfer()

7 Business Logic Review v2.2.0 to v3.0.0 Modules

As part of this audit we have further reviewed the business logic in version 2.2.0 of the LockUpTransferManager, BlacklistTransferManager and VestingEscrowWallet to ensure it matches the version 3.0.0 implementations. Polymath created the diffs and provided them to the audit team. They can be found here. Whilst we have audited that the business logic is functionally equivalent the correctness of these modules relies on the surrounding 2.2.0 code which has not been audited as part of this review. The following changes have been noted:

• Many changes are related to solidity 0.4.x vs. 0.5.x.
• Minor changes to delegator permissions OPERATOR and ADMIN.
• Insignificant visibility differences (public vs. external).
• Small internal functions have been removed and their logic is inlined instead.

Appendix 1 - File Hashes

The SHA1 hash of the source code files in scope of the audit are listed in the table below.

Appendix 2 - Severity

A.2.2 - Minor

Minor issues are generally subjective in nature, or potentially deal with topics like "best practices" or "readability". Minor issues in general will not indicate an actual problem or bug in code.

The maintainers should use their own judgment as to whether addressing these issues improves the codebase.

A.2.3 - Medium

Medium issues are generally objective in nature but do not represent actual bugs or security problems.

These issues should be addressed unless there is a clear reason not to.

A.2.4 - Major

Major issues will be things like bugs or security vulnerabilities. These issues may not be directly exploitable, or may require a certain condition to arise in order to be exploited.

Left unaddressed these issues are highly likely to cause problems with the operation of the contract or lead to a situation which allows the system to be exploited in some way.

A.2.5 - Critical

Critical issues are directly exploitable bugs or security vulnerabilities.

Left unaddressed these issues are highly likely or guaranteed to cause major problems or potentially a full failure in the operations of the contract.

Appendix 3 - 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 GitHub account (https://github.com/ConsenSys). 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.