From 030d640bbb1ad1796645cc296be435f65d41c195 Mon Sep 17 00:00:00 2001 From: Nikhil Parasaram Date: Sun, 29 Jan 2023 22:30:43 +0000 Subject: [PATCH] Fixes issue with ext calls (#1733) * Fix false positives on constructor * Fix issues with extcalls * Remove a log * Fix an ordering issue * Fix a bug with balance_ --- .../analysis/module/modules/unchecked_retval.py | 2 +- mythril/laser/ethereum/instructions.py | 13 ++++++++++--- mythril/laser/ethereum/state/world_state.py | 4 ++++ tests/integration_tests/analysis_tests.py | 15 +++++++++++++-- tests/testdata/input_contracts/extcall.sol | 15 +++++++++++++++ tests/testdata/inputs/extcall.sol.o | 1 + 6 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 tests/testdata/input_contracts/extcall.sol create mode 100644 tests/testdata/inputs/extcall.sol.o diff --git a/mythril/analysis/module/modules/unchecked_retval.py b/mythril/analysis/module/modules/unchecked_retval.py index f91428f9..b6a1e0ec 100644 --- a/mythril/analysis/module/modules/unchecked_retval.py +++ b/mythril/analysis/module/modules/unchecked_retval.py @@ -115,7 +115,7 @@ class UncheckedRetval(DetectionModule): ) conditions = [ And(*(state.world_state.constraints + [retval["retval"] == 1])), - And(*(state.world_state.constraints + [retval["retval"] == 1])), + And(*(state.world_state.constraints + [retval["retval"] == 0])), ] state.annotate( diff --git a/mythril/laser/ethereum/instructions.py b/mythril/laser/ethereum/instructions.py index 545f118f..6eec1807 100644 --- a/mythril/laser/ethereum/instructions.py +++ b/mythril/laser/ethereum/instructions.py @@ -912,11 +912,18 @@ class Instruction: """ state = global_state.mstate address = state.stack.pop() + onchain_access = True if address.symbolic is False: - balance = global_state.world_state.accounts_exist_or_load( - address.value, self.dynamic_loader - ).balance() + try: + balance = global_state.world_state.accounts_exist_or_load( + address.value, self.dynamic_loader + ).balance() + except ValueError: + onchain_access = False else: + onchain_access = False + + if onchain_access is False: balance = symbol_factory.BitVecVal(0, 256) for account in global_state.world_state.accounts.values(): balance = If(address == account.address, account.balance(), balance) diff --git a/mythril/laser/ethereum/state/world_state.py b/mythril/laser/ethereum/state/world_state.py index b4a90076..4efc3518 100644 --- a/mythril/laser/ethereum/state/world_state.py +++ b/mythril/laser/ethereum/state/world_state.py @@ -114,6 +114,9 @@ class WorldState: if dynamic_loader is None: raise ValueError("dynamic_loader is None") + if dynamic_loader.active is False: + raise ValueError("Dynamic Loader is deactivated. Use a symbol.") + if isinstance(addr, int): try: balance = dynamic_loader.read_balance("{0:#0{1}x}".format(addr, 42)) @@ -131,6 +134,7 @@ class WorldState: code = dynamic_loader.dynld(addr) except ValueError: code = None + return self.create_account( address=addr_bitvec.value, dynamic_loader=dynamic_loader, code=code ) diff --git a/tests/integration_tests/analysis_tests.py b/tests/integration_tests/analysis_tests.py index 49a33ab1..33a21856 100644 --- a/tests/integration_tests/analysis_tests.py +++ b/tests/integration_tests/analysis_tests.py @@ -40,13 +40,24 @@ test_data = ( }, None, ), + ( + "extcall.sol.o", + { + "TX_COUNT": 1, + "TX_OUTPUT": 0, + "MODULE": "Exceptions", + "ISSUE_COUNT": 1, + "ISSUE_NUMBER": 0, + }, + None, + ), ) @pytest.mark.parametrize("file_name, tx_data, calldata", test_data) def test_analysis(file_name, tx_data, calldata): bytecode_file = str(TESTDATA / "inputs" / file_name) - command = f"""python3 {MYTH} analyze -f {bytecode_file} -t {tx_data["TX_COUNT"]} -o jsonv2 -m {tx_data["MODULE"]} --solver-timeout 60000""" + command = f"""python3 {MYTH} analyze -f {bytecode_file} -t {tx_data["TX_COUNT"]} -o jsonv2 -m {tx_data["MODULE"]} --solver-timeout 60000 --no-onchain-data""" output = json.loads(output_of(command)) assert len(output[0]["issues"]) == tx_data["ISSUE_COUNT"] @@ -60,7 +71,7 @@ def test_analysis(file_name, tx_data, calldata): @pytest.mark.parametrize("file_name, tx_data, calldata", test_data) def test_analysis_delayed(file_name, tx_data, calldata): bytecode_file = str(TESTDATA / "inputs" / file_name) - command = f"""python3 {MYTH} analyze -f {bytecode_file} -t {tx_data["TX_COUNT"]} -o jsonv2 -m {tx_data["MODULE"]} --solver-timeout 60000 --strategy delayed""" + command = f"""python3 {MYTH} analyze -f {bytecode_file} -t {tx_data["TX_COUNT"]} -o jsonv2 -m {tx_data["MODULE"]} --solver-timeout 60000 --strategy delayed --no-onchain-data""" output = json.loads(output_of(command)) assert len(output[0]["issues"]) == tx_data["ISSUE_COUNT"] diff --git a/tests/testdata/input_contracts/extcall.sol b/tests/testdata/input_contracts/extcall.sol new file mode 100644 index 00000000..1408b5e7 --- /dev/null +++ b/tests/testdata/input_contracts/extcall.sol @@ -0,0 +1,15 @@ +pragma solidity ^0.6.0; + +interface IERC20 { + function transfer(address to, uint256 amount) external returns (bool); +} + +contract A { + constructor() public { + /// nothing detected + address(0).call(""); + IERC20(address(0)).transfer(address(0), 0); + assert(false); + } +} + diff --git a/tests/testdata/inputs/extcall.sol.o b/tests/testdata/inputs/extcall.sol.o new file mode 100644 index 00000000..ad6f903e --- /dev/null +++ b/tests/testdata/inputs/extcall.sol.o @@ -0,0 +1 @@ +608060405234801561001057600080fd5b50600073ffffffffffffffffffffffffffffffffffffffff166040518060000190506000604051808303816000865af19150503d806000811461006f576040519150601f19603f3d011682016040523d82523d6000602084013e610074565b606091505b505050600073ffffffffffffffffffffffffffffffffffffffff1663a9059cbb6000806040518363ffffffff1660e01b8152600401808373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200182815260200192505050602060405180830381600087803b15801561010057600080fd5b505af1158015610114573d6000803e3d6000fd5b505050506040513d602081101561012a57600080fd5b810190808051906020019092919050505050600061014457fe5b603f806101526000396000f3fe6080604052600080fdfea26469706673582212205bba7185aa846906fd4f2cc890c4095f521be787e49f5366712f751111e8d91764736f6c63430006000033 \ No newline at end of file