flex-dapps / embark-mythx

Status Embark plugin for MythX
MIT License
6 stars 6 forks source link

All source files are submitted as part of each contract analysis #11

Open emizzle opened 4 years ago

emizzle commented 4 years ago

If I were to add two completely separate contracts in my DApp (separate meaning that the contracts do not import or interact with each other at all), ie simple_storage.sol and reentrancy.sol, when I run the verify command, both contracts are submitted as separate MythX jobs, yet in each job, both files are analysed.

contracts/simple_storage.sol:

pragma solidity 0.4.15;

contract SimpleStorage {
    uint256 public storedData;

    event Set(address caller, uint256 _value);

    function SimpleStorage(uint256 initialValue) public {
        storedData = initialValue;
    }

    function set(uint256 x) public {
        storedData = x;
        Set(msg.sender, x);
    }

    function get() public constant returns (uint256 retVal) {
        return storedData;
    }
}

contracts/reentrancy.sol:

pragma solidity ^0.4.15;

contract Reentrance {
    mapping(address => uint256) userBalance;

    function getBalance(address u) constant returns (uint256) {
        return userBalance[u];
    }

    function addToBalance() payable {
        userBalance[msg.sender] += msg.value;
    }

    function withdrawBalance() {
        // send userBalance[msg.sender] ethers to msg.sender
        // if mgs.sender is a contract, it will call its fallback function
        if (!(msg.sender.call.value(userBalance[msg.sender])())) {
            throw;
        }
        userBalance[msg.sender] = 0;
    }

    function withdrawBalance_fixed() {
        // to protect against re-entrancy, the state variable
        // has to be change before the call
        uint256 amount = userBalance[msg.sender];
        userBalance[msg.sender] = 0;
        if (!(msg.sender.call.value(amount)())) {
            throw;
        }
    }

    function withdrawBalance_fixed_2() {
        // send() and transfer() are safe against reentrancy
        // they do not transfer the remaining gas
        // and they give just enough gas to execute few instructions
        // in the fallback function (no further call possible)
        msg.sender.transfer(userBalance[msg.sender]);
        userBalance[msg.sender] = 0;
    }

}

Running the verify command shows the following results: embark-mythx output

When I view the reentrancy.sol analysis in the MythX portal, I see that simple_storage.sol was submitted as part of that analysis: MythX Portal output for reentrancy.sol

Then, if we look at the simple_storage.sol analysis in the MythX portal, two things are apparent:

  1. reentrancy.sol was submitted as part of that analysis
  2. There are two medium warnings shown that were not displayed in the embark-mythx output MythX Portal analysis for simple_storage.sol
emizzle commented 4 years ago

Submitting a job with the correct sources was fixed in https://github.com/flex-dapps/embark-mythx/pull/12, however the result of this PR needs to be fully tested, and it also needs to be verified that the results in the MythX portal are correct (covered in #10).