Consensys / mythril

Security analysis tool for EVM bytecode. Supports smart contracts built for Ethereum, Hedera, Quorum, Vechain, Rootstock, Tron and other EVM-compatible blockchains.
https://mythx.io/
MIT License
3.88k stars 741 forks source link

`AttributeError: 'NoneType' object has no attribute 'get'` in `mythril/solidity/features.py` #1809

Closed abhinandanudupa closed 1 year ago

abhinandanudupa commented 1 year ago

Description

A Python error is thrown when analysing certain contracts. Earlier versions did not have this issue.

How to Reproduce

After installing (v0.24) from the PyPi (following the official documentation), run the following command analysing a certain contract (source code will be provided below):

myth a contract.sol

This is the error we get:

mythril.interfaces.cli [ERROR]: Traceback (most recent call last):
  File "~/virtualenv/lib/python3.11/site-packages/mythril/interfaces/cli.py", line 966, in parse_args_and_execute
    address = load_code(disassembler, args)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/interfaces/cli.py", line 717, in load_code
    address, _ = disassembler.load_from_solidity(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/mythril/mythril_disassembler.py", line 291, in load_from_solidity
    for contract in get_contracts_from_file(
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/soliditycontract.py", line 132, in get_contracts_from_file
    yield SolidityContract(
          ^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/soliditycontract.py", line 197, in __init__
    ).extract_features()
      ^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/features.py", line 28, in extract_features
    ether_vars = self.extract_address_variable(node)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/features.py", line 224, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(value))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/features.py", line 228, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(item))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/features.py", line 228, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(item))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/virtualenv/lib/python3.11/site-packages/mythril/solidity/features.py", line 210, in extract_address_variable
    node.get("nodeType", "") == "ExpressionStatement"
    ^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'get'

Source code of contract.sol:

pragma solidity ^0.8.0;

contract InsecureMoonToken {
    mapping (address => uint256) private userBalances;

    uint256 public constant TOKEN_PRICE = 1 ether;
    string public constant name = "Moon Token";
    string public constant symbol = "MOON";

    // The token is non-divisible
    // You can buy/sell/transfer 1, 2, 3, or 46 tokens but not 33.5
    uint8 public constant decimals = 0;

    uint256 public totalSupply;

    function buy(uint256 _amount) external payable {
        require(
            msg.value == _amount * TOKEN_PRICE, 
            "Ether submitted and Token amount to buy mismatch"
        );

        userBalances[msg.sender] += _amount;
        totalSupply += _amount;
    }

    function sell(uint256 _amount) external {
        require(userBalances[msg.sender] >= _amount, "Insufficient balance");

        userBalances[msg.sender] -= _amount;
        totalSupply -= _amount;

        (bool success, ) = msg.sender.call{value: _amount * TOKEN_PRICE}("");
        require(success, "Failed to send Ether");

        assert(getEtherBalance() == totalSupply * TOKEN_PRICE);
    }

    function transfer(address _to, uint256 _amount) external {
        require(_to != address(0), "_to address is not valid");
        require(userBalances[msg.sender] >= _amount, "Insufficient balance");

        userBalances[msg.sender] -= _amount;
        userBalances[_to] += _amount;
    }

    function getEtherBalance() public view returns (uint256) {
        return address(this).balance;
    }

    function getUserBalance(address _user) external view returns (uint256) {
        return userBalances[_user];
    }
}

Expected behavior

A successful execution with no errors and a number of detections.

Environment

Additional Notes

The same occurs with a lot of other contracts also.

abhinandanudupa commented 1 year ago

Looks like making this small change in the function extract_address_variable present in the file features.py:226-231 fixes this temporarily:

            elif isinstance(value, list):
                for item in value:
                    if item != None:
                        transfer_vars.update(self.extract_address_variable(item))

        return transfer_vars

(checking if item is not None instance and then updating)

abhinandanudupa commented 1 year ago

It turns out that objects likestrs (and ints) are also passed to the the function leading to the same issue. Is it due to some changes in an upstream package?

With:

/*
 * @author: Bernhard Mueller (ConsenSys / MythX)
 */

pragma solidity 0.6.4;

interface ICallable {
    function callMe() external;
}

contract HardcodedNotGood {

    address payable _callable = 0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaAa;
    ICallable callable = ICallable(_callable);

    constructor() public payable {
    }

    function doTransfer(uint256 amount) public {
        _callable.transfer(amount);
    }

    function doSend(uint256 amount) public {
        _callable.send(amount);
    }

     function callLowLevel() public {
         _callable.call.value(0).gas(10000)("");
     }

     function callWithArgs() public {
         callable.callMe{gas: 10000}();
     }
}

I get:

mythril.interfaces.cli [ERROR]: Traceback (most recent call last):
  File "~/venv/lib/python3.10/site-packages/mythril/interfaces/cli.py", line 966, in parse_args_and_execute
    address = load_code(disassembler, args)
  File "~/venv/lib/python3.10/site-packages/mythril/interfaces/cli.py", line 717, in load_code
    address, _ = disassembler.load_from_solidity(
  File "~/venv/lib/python3.10/site-packages/mythril/mythril/mythril_disassembler.py", line 291, in load_from_solidity
    for contract in get_contracts_from_file(
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/soliditycontract.py", line 132, in get_contracts_from_file
    yield SolidityContract(
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/soliditycontract.py", line 197, in __init__
    ).extract_features()
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 28, in extract_features
    ether_vars = self.extract_address_variable(node)
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 224, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(value))
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 228, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(item))
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 224, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(value))
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 224, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(value))
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 228, in extract_address_variable
    transfer_vars.update(self.extract_address_variable(item))
  File "~/venv/lib/python3.10/site-packages/mythril/solidity/features.py", line 210, in extract_address_variable
    node.get("nodeType", "") == "ExpressionStatement"
AttributeError: 'str' object has no attribute 'get'

Sorry for the delay in reporting this.

norhh commented 1 year ago

Does the current develop work for you? This example works for me on the develop. Clone the develop and you can directly execute it using ./myth analyze

abhinandanudupa commented 1 year ago

Yes, surprisingly it did fix that issue. But I found a new one in a different file:

mythril.interfaces.cli [ERROR]: Traceback (most recent call last):
  File "/home/parakeet/tests/mythril/mythril/interfaces/cli.py", line 966, in parse_args_and_execute
    address = load_code(disassembler, args)
  File "/home/parakeet/tests/mythril/mythril/interfaces/cli.py", line 717, in load_code
    address, _ = disassembler.load_from_solidity(
  File "/home/parakeet/tests/mythril/mythril/mythril/mythril_disassembler.py", line 272, in load_from_solidity
    if self.check_run_integer_module(file) is False:
  File "/home/parakeet/tests/mythril/mythril/mythril/mythril_disassembler.py", line 236, in check_run_integer_module
    normalized_version = self.solc_version.lstrip("v")
AttributeError: 'NoneType' object has no attribute 'lstrip'

The contract in question does not specify the solidity version (which is wrong) but when I tested it with Mythril v0.23.19 the compilation works fine.

Looks like the source is this recent change: https://github.com/Consensys/mythril/commit/a28d600ced16e1ae25d49dda571028581c32929e

norhh commented 1 year ago

This mythril version tries to autofind the solidity version, Can you try to paste the first few lines at the top of the file (till contract <ContractName>)? I'll try fixing this issue

abhinandanudupa commented 1 year ago

Here is the contract:

contract MarketPlace {
    address owner;
    uint256 private cost = 100;
    uint256 private inventory = 20;
    event Purchase(address _buyer, uint256 _amt);

    function increasePrice (uint256 increaseCost) public {
        require(msg.sender == owner);
        cost += increaseCost;
    }

    function buy() public payable returns (uint256) {
        uint256 amt = msg.value / cost;
        require(inventory > amt);
        inventory -= amt;
        emit Purchase(msg.sender, amt);
    }
}
norhh commented 1 year ago

Thanks for the contract, the PR #1812 will fix this