diff --git a/mythril/solidity/features.py b/mythril/solidity/features.py index 6fb03c6a..6a666bed 100644 --- a/mythril/solidity/features.py +++ b/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 ( diff --git a/tests/features_test.py b/tests/features_test.py index a89d1148..99cfcd88 100644 --- a/tests/features_test.py +++ b/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), - ("rubixi.sol", 18, "collectAllFees", "has_owner_modifier", True), - ("rubixi.sol", 18, "collectAllFees", "is_payable", False), + ("rubixi.sol", 18, "init", "contains_selfdestruct", False, solc_binary_5), + ("rubixi.sol", 18, "collectAllFees", "has_owner_modifier", True, solc_binary_5), + ("rubixi.sol", 18, "collectAllFees", "is_payable", False, solc_binary_5), ( "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(): diff --git a/tests/testdata/input_contracts/regression_1.sol b/tests/testdata/input_contracts/regression_1.sol new file mode 100644 index 00000000..73d9d572 --- /dev/null +++ b/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]; + } +} \ No newline at end of file