Handle edge case during feature extraction (#1811)

* Update requirements.txt

* use eth-abi

* Update requirements.txt

* Handle edge case during feature extraction

mythril/solidity/features.py

@@ -203,7 +203,7 @@ class SolidityFeatureExtractor:
         return variables
 
     def extract_address_variable(self, node):
-        if isinstance(node, int):
+        if node is None or isinstance(node, (int, str)):
             return set([])
         transfer_vars = set([])
         if (

tests/features_test.py

@@ -5,27 +5,38 @@ from mythril.solidity.features import SolidityFeatureExtractor
 from mythril.solidity.soliditycontract import SolidityContract, SolcAST
 
 TEST_FILES = Path(__file__).parent / "testdata/input_contracts"
-solc_binary = MythrilDisassembler._init_solc_binary("v0.5.0")
+solc_binary_5 = MythrilDisassembler._init_solc_binary("v0.5.0")
+solc_binary_8 = MythrilDisassembler._init_solc_binary("v0.8.0")
+
 
 test_cases = [
-    ("suicide.sol", 1, "kill", "contains_selfdestruct", True),
+    ("suicide.sol", 1, "kill", "contains_selfdestruct", True, solc_binary_5),
     (
         "SimpleModifier.sol",
         1,
         "withdrawfunds",
         "all_require_vars",
         set(["msg", "owner"]),
+        solc_binary_5,
+    ),
+    (
+        "SimpleModifier.sol",
+        1,
+        "withdrawfunds",
+        "transfer_vars",
+        set(["owner"]),
+        solc_binary_5,
     ),
-    ("SimpleModifier.sol", 1, "withdrawfunds", "transfer_vars", set(["owner"])),
+    ("rubixi.sol", 18, "init", "contains_selfdestruct", False, solc_binary_5),
-    ("rubixi.sol", 18, "init", "contains_selfdestruct", False),
+    ("rubixi.sol", 18, "collectAllFees", "has_owner_modifier", True, solc_binary_5),
-    ("rubixi.sol", 18, "collectAllFees", "has_owner_modifier", True),
+    ("rubixi.sol", 18, "collectAllFees", "is_payable", False, solc_binary_5),
-    ("rubixi.sol", 18, "collectAllFees", "is_payable", False),
     (
         "rubixi.sol",
         18,
         "collectAllFees",
         "all_require_vars",
         set(["collectedFees", "creator"]),
+        solc_binary_5,
     ),
     (
         "rubixi.sol",
@@ -33,6 +44,7 @@ test_cases = [
         "collectPercentOfFees",
         "all_require_vars",
         set(["collectedFees", "_pcent", "creator"]),
+        solc_binary_5,
     ),
     (
         "rubixi.sol",
@@ -40,25 +52,62 @@ test_cases = [
         "changeMultiplier",
         "all_require_vars",
         set(["_mult", "creator"]),
+        solc_binary_5,
+    ),
+    ("rubixi.sol", 18, "", "is_payable", True, solc_binary_5),
+    (
+        "rubixi.sol",
+        18,
+        "collectAllFees",
+        "transfer_vars",
+        set(["creator"]),
+        solc_binary_5,
+    ),
+    ("exceptions.sol", 8, "assert3", "contains_assert", True, solc_binary_5),
+    (
+        "exceptions.sol",
+        8,
+        "requireisfine",
+        "all_require_vars",
+        set(["input"]),
+        solc_binary_5,
+    ),
+    ("WalletLibrary.sol", 23, "execute", "has_owner_modifier", True, solc_binary_5),
+    ("WalletLibrary.sol", 23, "initWallet", "has_owner_modifier", False, solc_binary_5),
+    (
+        "WalletLibrary.sol",
+        23,
+        "initWallet",
+        "all_require_vars",
+        set(["m_numOwners"]),
+        solc_binary_5,
+    ),
+    (
+        "WalletLibrary.sol",
+        23,
+        "confirm",
+        "all_require_vars",
+        set(["success"]),
+        solc_binary_5,
+    ),
+    ("kcalls.sol", 3, "callSetN", "contains_call", True, solc_binary_5),
+    ("kcalls.sol", 3, "delegatecallSetN", "contains_delegatecall", True, solc_binary_5),
+    ("kcalls.sol", 3, "callcodeSetN", "contains_staticcall", True, solc_binary_5),
+    (
+        "regression_1.sol",
+        5,
+        "transfer",
+        "all_require_vars",
+        set(["userBalances", "msg", "_to", "_amount"]),
+        solc_binary_8,
     ),
-    ("rubixi.sol", 18, "", "is_payable", True),
-    ("rubixi.sol", 18, "collectAllFees", "transfer_vars", set(["creator"])),
-    ("exceptions.sol", 8, "assert3", "contains_assert", True),
-    ("exceptions.sol", 8, "requireisfine", "all_require_vars", set(["input"])),
-    ("WalletLibrary.sol", 23, "execute", "has_owner_modifier", True),
-    ("WalletLibrary.sol", 23, "initWallet", "has_owner_modifier", False),
-    ("WalletLibrary.sol", 23, "initWallet", "all_require_vars", set(["m_numOwners"])),
-    ("WalletLibrary.sol", 23, "confirm", "all_require_vars", set(["success"])),
-    ("kcalls.sol", 3, "callSetN", "contains_call", True),
-    ("kcalls.sol", 3, "delegatecallSetN", "contains_delegatecall", True),
-    ("kcalls.sol", 3, "callcodeSetN", "contains_staticcall", True),
 ]
 
 
 @pytest.mark.parametrize(
-    "file_name, num_funcs, func_name, field, expected_value", test_cases
+    "file_name, num_funcs, func_name, field, expected_value, solc_binary", test_cases
 )
-def test_feature_selfdestruct(file_name, num_funcs, func_name, field, expected_value):
+def test_features(file_name, num_funcs, func_name, field, expected_value, solc_binary):
     input_file = TEST_FILES / file_name
     name = file_name.split(".")[0]
     if name[0].islower():

tests/testdata/input_contracts/regression_1.sol

@@ -0,0 +1,53 @@
+pragma solidity ^0.8.0;
+
+contract Regression_1 {
+    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];
+    }
+}