diff --git a/mythril/analysis/modules/suicide.py b/mythril/analysis/modules/suicide.py index f0db917e..49794215 100644 --- a/mythril/analysis/modules/suicide.py +++ b/mythril/analysis/modules/suicide.py @@ -79,7 +79,7 @@ def execute(statespace): break - # CALLER may also be constrained to hardcoded address. I.e. 'caller' and some integer + # CALLER may also be constrained to hardcoded address. I.e. 'caller' and some integer elif (re.search(r"caller", str(constraint)) and re.search(r'[0-9]{20}', str(constraint))): can_solve = False diff --git a/mythril/analysis/modules/unchecked_retval.py b/mythril/analysis/modules/unchecked_retval.py index 69d997c4..aa0f4022 100644 --- a/mythril/analysis/modules/unchecked_retval.py +++ b/mythril/analysis/modules/unchecked_retval.py @@ -1,9 +1,7 @@ -from z3 import * -import re -from mythril.analysis.ops import * from mythril.analysis.report import Issue +from laser.ethereum.svm import NodeFlags import logging -from laser.ethereum import helper +import re ''' @@ -30,55 +28,67 @@ def execute(statespace): logging.debug("Executing module: UNCHECKED_RETVAL") issues = [] - visited = [] - for call in statespace.calls: + for k in statespace.nodes: + + node = statespace.nodes[k] + + if NodeFlags.CALL_RETURN in node.flags: + + retval_checked = False + + for state in node.states: - state = call.state - address = state.get_current_instruction()['address'] + instr = state.get_current_instruction() - # Only needs to be checked once per call instructions (it's essentially just static analysis) + if (instr['opcode'] == 'ISZERO' and re.search(r'retval', str(state.mstate.stack[-1]))): + retval_checked = True + break + + if not retval_checked: + + address = state.get_current_instruction()['address'] + issue = Issue(node.contract_name, node.function_name, address, "Unchecked CALL return value") + + issue.description = \ + "The return value of an external call is not checked. Note that the contract will continue if the call fails." + + issues.append(issue) - if call.state.mstate.pc in visited: - continue else: - visited.append(call.state.mstate.pc) - retval_checked = False + nStates = len(node.states) + + for idx in range(0, nStates): - # ISZERO retval is expected to be found within the next few instructions. + state = node.states[idx] + instr = state.get_current_instruction() - for i in range(0, 10): + if (instr['opcode'] == 'CALL'): - _state = call.node.states[call.state_index + i] + retval_checked = False - try: - instr = _state.get_current_instruction() - except IndexError: - break + for _idx in range(idx, idx + 10): - if (instr['opcode'] == 'ISZERO' and re.search(r'retval', str(_state.mstate.stack[-1]))): - retval_checked = True - break + try: + _state = node.states[_idx] + _instr = _state.get_current_instruction() - if not retval_checked: + if (_instr['opcode'] == 'ISZERO' and re.search(r'retval', str(_state .mstate.stack[-1]))): + retval_checked = True + break - issue = Issue(call.node.contract_name, call.node.function_name, address, "Unchecked CALL return value") + except IndexError: + break - if (call.to.type == VarType.CONCRETE): - receiver = hex(call.to.val) - elif (re.search(r"caller", str(call.to))): - receiver = "msg.sender" - elif (re.search(r"storage", str(call.to))): - receiver = "an address obtained from storage" - else: - receiver = str(call.to) + if not retval_checked: + address = instr['address'] + issue = Issue(node.contract_name, node.function_name, address, "Unchecked CALL return value") - issue.description = \ - "The function " + call.node.function_name + " contains a call to " + receiver + ".\n" \ - "The return value of this call is not checked. Note that the function will continue to execute with a return value of '0' if the called contract throws." + issue.description = \ + "The return value of an external call is not checked. Note that execution continue even if the called contract throws." - issues.append(issue) + issues.append(issue) - return issues \ No newline at end of file + return issues diff --git a/solidity_examples/reentrancy.sol b/solidity_examples/reentrancy.sol index 78210aa8..357152a6 100644 --- a/solidity_examples/reentrancy.sol +++ b/solidity_examples/reentrancy.sol @@ -12,9 +12,7 @@ contract Reentrancy { function withdraw(uint _amount) public { if(balances[msg.sender] >= _amount) { - if(msg.sender.call.value(_amount)()) { - _amount; - } + msg.sender.call.value(_amount)(); balances[msg.sender] -= _amount; } } diff --git a/solidity_examples/returnvalue.sol b/solidity_examples/returnvalue.sol new file mode 100644 index 00000000..0f7b5fd4 --- /dev/null +++ b/solidity_examples/returnvalue.sol @@ -0,0 +1,19 @@ +contract ReturnValue { + + address callee = 0xE0F7e56e62b4267062172495D7506087205A4229; + + function call1() returns (uint256) { + callee.call(); + } + + function call2() returns (uint256) { + + + } + + function call3() returns (uint256) { + + + } + +} \ No newline at end of file