diff --git a/slither/detectors/functions/arbitrary_send.py b/slither/detectors/functions/arbitrary_send.py index 920351d41..c72cc84c0 100644 --- a/slither/detectors/functions/arbitrary_send.py +++ b/slither/detectors/functions/arbitrary_send.py @@ -97,24 +97,24 @@ class ArbitrarySend(AbstractDetector): for c in self.contracts: arbitrary_send = self.detect_arbitrary_send(c) for (func, nodes) in arbitrary_send: - func_name = func.name calls_str = [str(node.expression) for node in nodes] - txt = "Arbitrary send in {} Contract: {}, Function: {}, Calls: {}" - info = txt.format(self.filename, - c.name, - func_name, - calls_str) + info = "{}{} sends eth to arbirary user\n" + info = info.format(func.contract.name, + func.name) + info += '\tDangerous calls:\n' + for node in nodes: + info += '- {} ({})'.format(node.expression, node.source_mapping_str) self.log(info) source_mapping = [node.source_mapping for node in nodes] - results.append({'vuln': 'SuicidalFunc', + results.append({'vuln': 'ArbitrarySend', 'sourceMapping': source_mapping, 'filename': self.filename, - 'contract': c.name, - 'func': func_name, + 'contract': func.contract.name, + 'func': func.name, 'calls': calls_str}) return results diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 929414c71..595634a88 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -56,7 +56,7 @@ class ExternalFunction(AbstractDetector): for func in [f for f in contract.functions if f.visibility == 'public' and\ not f in public_function_calls and\ not f.is_constructor]: - txt = "{}.{} ({}) should be declared external" + txt = "{}.{} ({}) should be declared external\n" info = txt.format(func.contract.name, func.name, func.source_mapping_str) diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index f3312c1f0..ebf5e8c0b 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -45,7 +45,7 @@ class NamingConvention(AbstractDetector): for contract in self.contracts: if not self.is_cap_words(contract.name): - info = "Contract '{}' is not in CapWords".format(contract.name) + info = "Contract '{}' ({}) is not in CapWords\n".format(contract.name, contract.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -58,7 +58,8 @@ class NamingConvention(AbstractDetector): continue if not self.is_cap_words(struct.name): - info = "Struct '{}' is not in CapWords, Contract: '{}' ".format(struct.name, contract.name) + info = "Struct '{}.{}' ({}) is not in CapWords\n" + info = info.format(struct.contract.name, struct.name, struct.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -72,7 +73,8 @@ class NamingConvention(AbstractDetector): continue if not self.is_cap_words(event.name): - info = "Event '{}' is not in CapWords, Contract: '{}' ".format(event.name, contract.name) + info = "Event '{}.{}' ({}) is not in CapWords\n" + info = info.format(event.contract.name, event.name, event.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -86,7 +88,8 @@ class NamingConvention(AbstractDetector): continue if not self.is_mixed_case(func.name): - info = "Function '{}' is not in mixedCase, Contract: '{}' ".format(func.name, contract.name) + info = "Function '{}.{}' ({}) is not in mixedCase\n" + info = info.format(func.contract.name, func.name, func.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -101,8 +104,11 @@ class NamingConvention(AbstractDetector): else: correct_naming = self.is_mixed_case_with_underscore(argument.name) if not correct_naming: - info = "Parameter '{}' is not in mixedCase, Contract: '{}', Function: '{}'' " \ - .format(argument.name, argument.name, contract.name) + info = "Parameter '{}' of {}.{} ({}) is not in mixedCase\n" + info = info.format(argument.name, + argument.function.contract.name, + argument.function, + argument.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -118,8 +124,8 @@ class NamingConvention(AbstractDetector): if self.should_avoid_name(var.name): if not self.is_upper_case_with_underscores(var.name): - info = "Variable '{}' l, O, I should not be used, Contract: '{}' " \ - .format(var.name, contract.name) + info = "Variable '{}.{}' ({}) used l, O, I, which should not be used\n" + info = info.format(var.contract.name, var.name, var.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -134,8 +140,8 @@ class NamingConvention(AbstractDetector): continue if not self.is_upper_case_with_underscores(var.name): - info = "Constant '{}' is not in UPPER_CASE_WITH_UNDERSCORES, Contract: '{}' " \ - .format(var.name, contract.name) + info = "Constant '{}.{}' ({}) is not in UPPER_CASE_WITH_UNDERSCORES\n" + info = info.format(var.contract.name, var.name, var.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -149,7 +155,8 @@ class NamingConvention(AbstractDetector): else: correct_naming = self.is_mixed_case(var.name) if not correct_naming: - info = "Variable '{}' is not in mixedCase, Contract: '{}' ".format(var.name, contract.name) + info = "Variable '{}.{}' ({}) is not in mixedCase" + info = info.format(var.contract.name, var.name, var.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -163,7 +170,8 @@ class NamingConvention(AbstractDetector): continue if not self.is_cap_words(enum.name): - info = "Enum '{}' is not in CapWords, Contract: '{}' ".format(enum.name, contract.name) + info = "Enum '{}.{}' ({}) is not in CapWords\n" + info = info.format(enum.contract.name, enum.name, enum.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', @@ -177,7 +185,8 @@ class NamingConvention(AbstractDetector): continue if not self.is_mixed_case(modifier.name): - info = "Modifier '{}' is not in mixedCase, Contract: '{}' ".format(modifier.name, contract.name) + info = "Modifier '{}.{}' ({}) is not in mixedCase\n" + info = info.format(modifier.contract.name, modifier.name, modifier.source_mapping_str) self.log(info) results.append({'vuln': 'NamingConvention', diff --git a/slither/detectors/operations/low_level_calls.py b/slither/detectors/operations/low_level_calls.py index ec0f546a7..a5a7e828a 100644 --- a/slither/detectors/operations/low_level_calls.py +++ b/slither/detectors/operations/low_level_calls.py @@ -27,7 +27,7 @@ class LowLevelCalls(AbstractDetector): def detect_low_level_calls(self, contract): ret = [] - for f in contract.functions: + for f in [f for f in contract.functions if contract == f.contract]: nodes = f.nodes assembly_nodes = [n for n in nodes if self._contains_low_level_calls(n)] @@ -42,10 +42,8 @@ class LowLevelCalls(AbstractDetector): for c in self.contracts: values = self.detect_low_level_calls(c) for func, nodes in values: - func_name = func.name - info = "Low level call in %s, Contract: %s, Function: %s" % (self.filename, - c.name, - func_name) + info = "Low level call in {}.{} ({})" + info = info.format(func.contract.name, func.name, func.source_mapping_str) self.log(info) sourceMapping = [n.source_mapping for n in nodes] @@ -53,7 +51,7 @@ class LowLevelCalls(AbstractDetector): results.append({'vuln': 'Low level call', 'sourceMapping': sourceMapping, 'filename': self.filename, - 'contract': c.name, - 'function_name': func_name}) + 'contract': func.contract.name, + 'function_name': func.name}) return results diff --git a/slither/detectors/reentrancy/reentrancy.py b/slither/detectors/reentrancy/reentrancy.py index e5c9966a7..2d6c63cb7 100644 --- a/slither/detectors/reentrancy/reentrancy.py +++ b/slither/detectors/reentrancy/reentrancy.py @@ -126,7 +126,7 @@ class Reentrancy(AbstractDetector): if isinstance(internal_call, Function): state_vars_written += internal_call.all_state_variables_written() - read_then_written = [v for v in state_vars_written if v in node.context[self.key]['read']] + read_then_written = [(v, node.source_mapping_str) for v in state_vars_written if v in node.context[self.key]['read']] node.context[self.key]['read'] = list(set(node.context[self.key]['read'] + node.state_variables_read)) # If a state variables was read and is then written, there is a dangerous call and @@ -136,8 +136,7 @@ class Reentrancy(AbstractDetector): node.context[self.key]['calls'] and node.context[self.key]['send_eth']): # calls are ordered - finding_key = (node.function.contract.name, - node.function.full_name, + finding_key = (node.function, tuple(set(node.context[self.key]['calls'])), tuple(set(node.context[self.key]['send_eth']))) finding_vars = read_then_written @@ -176,32 +175,38 @@ class Reentrancy(AbstractDetector): results = [] - for (contract, func, calls, send_eth), varsWritten in self.result.items(): - varsWritten_str = list(set([str(x) for x in list(varsWritten)])) - calls_str = list(set([str(x.expression) for x in list(calls)])) - send_eth_str = list(set([str(x.expression) for x in list(send_eth)])) - - if calls == send_eth: - call_info = 'Call: {},'.format(calls_str) - else: - call_info = 'Call: {}, Ether sent: {},'.format(calls_str, send_eth_str) - info = 'Reentrancy in {}, Contract: {}, '.format(self.filename, contract) + \ - 'Func: {}, '.format(func) + \ - '{}'.format(call_info) + \ - 'Vars Written: {}'.format(str(varsWritten_str)) + for (func, calls, send_eth), varsWritten in self.result.items(): + calls = list(set(calls)) + send_eth = list(set(send_eth)) +# if calls == send_eth: +# calls_info = 'Call: {},'.format(calls_str) +# else: +# calls_info = 'Call: {}, Ether sent: {},'.format(calls_str, send_eth_str) + info = 'Reentrancy in {}.{} ({}):\n' + info = info.format(func.contract.name, func.name, func.source_mapping_str) + info += '\tExternal calls:\n' + for call_info in calls: + info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str) + if calls != send_eth: + info += '\tExternal calls sending eth:\n' + for call_info in send_eth: + info += '\t- {} ({})\n'.format(call_info.expression, call_info.source_mapping_str) + info += '\tState variables written after the call(s):\n' + for (v, mapping) in varsWritten: + info += '\t- {} ({})\n'.format(v, mapping) self.log(info) - source = [v.source_mapping for v in varsWritten] + source = [v.source_mapping for (v,_) in varsWritten] source += [node.source_mapping for node in calls] source += [node.source_mapping for node in send_eth] results.append({'vuln': 'Reentrancy', 'sourceMapping': source, 'filename': self.filename, - 'contract': contract, - 'function_name': func, - 'calls': calls_str, - 'send_eth': send_eth_str, - 'varsWritten': varsWritten_str}) + 'contract': func.contract.name, + 'function_name': func.name, + 'calls': [str(x.expression) for x in calls], + 'send_eth': [str(x.expression) for x in send_eth], + 'varsWritten': [str(x) for (x,_) in varsWritten]}) return results diff --git a/slither/detectors/statements/assembly.py b/slither/detectors/statements/assembly.py index 93e9ea598..f2bd62a8b 100644 --- a/slither/detectors/statements/assembly.py +++ b/slither/detectors/statements/assembly.py @@ -42,10 +42,8 @@ class Assembly(AbstractDetector): for c in self.contracts: values = self.detect_assembly(c) for func, nodes in values: - func_name = func.name - info = "Assembly in %s, Contract: %s, Function: %s" % (self.filename, - c.name, - func_name) + info = "{}.{} uses assembly ({})\n" + info = info.format(func.contract.name, func.name, func.source_mapping_str) self.log(info) sourceMapping = [n.source_mapping for n in nodes] @@ -53,7 +51,7 @@ class Assembly(AbstractDetector): results.append({'vuln': 'Assembly', 'sourceMapping': sourceMapping, 'filename': self.filename, - 'contract': c.name, - 'function_name': func_name}) + 'contract': func.contract.name, + 'function_name': func.name}) return results diff --git a/slither/detectors/statements/tx_origin.py b/slither/detectors/statements/tx_origin.py index f5c474f8f..fb14bdfba 100644 --- a/slither/detectors/statements/tx_origin.py +++ b/slither/detectors/statements/tx_origin.py @@ -48,10 +48,12 @@ class TxOrigin(AbstractDetector): for c in self.contracts: values = self.detect_tx_origin(c) for func, nodes in values: - func_name = func.name - info = "tx.origin in %s, Contract: %s, Function: %s" % (self.filename, - c.name, - func_name) + info = "{}.{} uses tx.origin for authorization:\n" + info = info.format(func.contract.name, func.name) + + for node in nodes: + info += "\t- {} ({})\n".format(node.expression, node.source_mapping_str) + self.log(info) sourceMapping = [n.source_mapping for n in nodes] @@ -59,7 +61,7 @@ class TxOrigin(AbstractDetector): results.append({'vuln': 'TxOrigin', 'sourceMapping': sourceMapping, 'filename': self.filename, - 'contract': c.name, - 'function_name': func_name}) + 'contract': func.contract.name, + 'function_name': func.name}) return results diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 087750577..92445e03e 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -218,6 +218,7 @@ class ContractSolc04(Contract): event = EventSolc(event_to_parse, self) event.analyze(self) event.set_contract(self) + event.set_offset(event_to_parse['src'], self.slither) self._events[event.full_name] = event self._eventsNotParsed = None diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 4a2e5d926..b138f0d06 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -92,8 +92,9 @@ class FunctionSolc(Function): if 'payable' in attributes: self._payable = attributes['payable'] - def _new_node(self, node_type): + def _new_node(self, node_type, src): node = NodeSolc(node_type, self._counter_nodes) + node.set_offset(src, self.slither) self._counter_nodes += 1 node.set_function(self) self._nodes.append(node) @@ -107,7 +108,7 @@ class FunctionSolc(Function): condition = ifStatement['condition'] # Note: check if the expression could be directly # parsed here - condition_node = self._new_node(NodeType.IF) + condition_node = self._new_node(NodeType.IF, ifStatement['src']) condition_node.add_unparsed_expression(condition) link_nodes(node, condition_node) trueStatement = self._parse_statement(ifStatement['trueBody'], condition_node) @@ -118,14 +119,14 @@ class FunctionSolc(Function): condition = children[0] # Note: check if the expression could be directly # parsed here - condition_node = self._new_node(NodeType.IF) + condition_node = self._new_node(NodeType.IF, ifStatement['src']) condition_node.add_unparsed_expression(condition) link_nodes(node, condition_node) trueStatement = self._parse_statement(children[1], condition_node) if len(children) == 3: falseStatement = self._parse_statement(children[2], condition_node) - endIf_node = self._new_node(NodeType.ENDIF) + endIf_node = self._new_node(NodeType.ENDIF, ifStatement['src']) link_nodes(trueStatement, endIf_node) if falseStatement: @@ -164,8 +165,8 @@ class FunctionSolc(Function): def _parse_while(self, whileStatement, node): # WhileStatement = 'while' '(' Expression ')' Statement - node_startWhile = self._new_node(NodeType.STARTLOOP) - node_condition = self._new_node(NodeType.IFLOOP) + node_startWhile = self._new_node(NodeType.STARTLOOP, whileStatement['src']) + node_condition = self._new_node(NodeType.IFLOOP, whileStatement['src']) if self.is_compact_ast: node_condition.add_unparsed_expression(whileStatement['condition']) @@ -176,7 +177,7 @@ class FunctionSolc(Function): node_condition.add_unparsed_expression(expression) statement = self._parse_statement(children[1], node_condition) - node_endWhile = self._new_node(NodeType.ENDLOOP) + node_endWhile = self._new_node(NodeType.ENDLOOP, whileStatement['src']) link_nodes(node, node_startWhile) link_nodes(node_startWhile, node_condition) @@ -191,8 +192,8 @@ class FunctionSolc(Function): condition = statement['condition'] loop_expression = statement['loopExpression'] - node_startLoop = self._new_node(NodeType.STARTLOOP) - node_endLoop = self._new_node(NodeType.ENDLOOP) + node_startLoop = self._new_node(NodeType.STARTLOOP, statement['src']) + node_endLoop = self._new_node(NodeType.ENDLOOP, statement['src']) if init_expression: node_init_expression = self._parse_statement(init_expression, node) @@ -201,7 +202,7 @@ class FunctionSolc(Function): link_nodes(node, node_startLoop) if condition: - node_condition = self._new_node(NodeType.IFLOOP) + node_condition = self._new_node(NodeType.IFLOOP, statement['src']) node_condition.add_unparsed_expression(condition) link_nodes(node_startLoop, node_condition) link_nodes(node_condition, node_endLoop) @@ -254,8 +255,8 @@ class FunctionSolc(Function): hasLoopExpression = False - node_startLoop = self._new_node(NodeType.STARTLOOP) - node_endLoop = self._new_node(NodeType.ENDLOOP) + node_startLoop = self._new_node(NodeType.STARTLOOP, statement['src']) + node_endLoop = self._new_node(NodeType.ENDLOOP, statement['src']) children = statement[self.get_children('children')] @@ -283,7 +284,7 @@ class FunctionSolc(Function): if candidate[self.get_key()] not in ['VariableDefinitionStatement', 'VariableDeclarationStatement', 'ExpressionStatement']: - node_condition = self._new_node(NodeType.IFLOOP) + node_condition = self._new_node(NodeType.IFLOOP, statement['src']) #expression = parse_expression(candidate, self) expression = candidate node_condition.add_unparsed_expression(expression) @@ -313,8 +314,8 @@ class FunctionSolc(Function): def _parse_dowhile(self, doWhilestatement, node): - node_startDoWhile = self._new_node(NodeType.STARTLOOP) - node_condition = self._new_node(NodeType.IFLOOP) + node_startDoWhile = self._new_node(NodeType.STARTLOOP, doWhilestatement['src']) + node_condition = self._new_node(NodeType.IFLOOP, doWhilestatement['src']) if self.is_compact_ast: node_condition.add_unparsed_expression(doWhilestatement['condition']) @@ -326,7 +327,7 @@ class FunctionSolc(Function): node_condition.add_unparsed_expression(expression) statement = self._parse_statement(children[1], node_condition) - node_endDoWhile = self._new_node(NodeType.ENDLOOP) + node_endDoWhile = self._new_node(NodeType.ENDLOOP, doWhilestatement['src']) link_nodes(node, node_startDoWhile) link_nodes(node_startDoWhile, statement) @@ -344,7 +345,7 @@ class FunctionSolc(Function): self._variables[local_var.name] = local_var #local_var.analyze(self) - new_node = self._new_node(NodeType.VARIABLE) + new_node = self._new_node(NodeType.VARIABLE, statement['src']) new_node.add_variable_declaration(local_var) link_nodes(node, new_node) return new_node @@ -418,7 +419,7 @@ class FunctionSolc(Function): 'typeDescriptions': {'typeString':'tuple()'} } node = new_node - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) @@ -490,7 +491,7 @@ class FunctionSolc(Function): self.get_children('children'): var_identifiers}, tuple_vars]} node = new_node - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) @@ -506,7 +507,7 @@ class FunctionSolc(Function): self._variables[local_var.name] = local_var # local_var.analyze(self) - new_node = self._new_node(NodeType.VARIABLE) + new_node = self._new_node(NodeType.VARIABLE, statement['src']) new_node.add_variable_declaration(local_var) link_nodes(node, new_node) return new_node @@ -534,7 +535,7 @@ class FunctionSolc(Function): elif name == 'Block': node = self._parse_block(statement, node) elif name == 'InlineAssembly': - break_node = self._new_node(NodeType.ASSEMBLY) + break_node = self._new_node(NodeType.ASSEMBLY, statement['src']) link_nodes(node, break_node) node = break_node elif name == 'DoWhileStatement': @@ -542,15 +543,15 @@ class FunctionSolc(Function): # For Continue / Break / Return / Throw # The is fixed later elif name == 'Continue': - continue_node = self._new_node(NodeType.CONTINUE) + continue_node = self._new_node(NodeType.CONTINUE, statement['src']) link_nodes(node, continue_node) node = continue_node elif name == 'Break': - break_node = self._new_node(NodeType.BREAK) + break_node = self._new_node(NodeType.BREAK, statement['src']) link_nodes(node, break_node) node = break_node elif name == 'Return': - return_node = self._new_node(NodeType.RETURN) + return_node = self._new_node(NodeType.RETURN, statement['src']) link_nodes(node, return_node) if self.is_compact_ast: if statement['expression']: @@ -562,7 +563,7 @@ class FunctionSolc(Function): return_node.add_unparsed_expression(expression) node = return_node elif name == 'Throw': - throw_node = self._new_node(NodeType.THROW) + throw_node = self._new_node(NodeType.THROW, statement['src']) link_nodes(node, throw_node) node = throw_node elif name == 'EmitStatement': @@ -571,7 +572,7 @@ class FunctionSolc(Function): expression = statement['eventCall'] else: expression = statement[self.get_children('children')][0] - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) node = new_node @@ -585,7 +586,7 @@ class FunctionSolc(Function): expression = statement[self.get_children('expression')] else: expression = statement[self.get_children('expression')][0] - new_node = self._new_node(NodeType.EXPRESSION) + new_node = self._new_node(NodeType.EXPRESSION, statement['src']) new_node.add_unparsed_expression(expression) link_nodes(node, new_node) node = new_node @@ -615,7 +616,7 @@ class FunctionSolc(Function): assert cfg[self.get_key()] == 'Block' - node = self._new_node(NodeType.ENTRYPOINT) + node = self._new_node(NodeType.ENTRYPOINT, cfg['src']) self._entry_point = node if self.is_compact_ast: @@ -866,23 +867,23 @@ class FunctionSolc(Function): def split_ternary_node(self, node, condition, true_expr, false_expr): - condition_node = self._new_node(NodeType.IF) + condition_node = self._new_node(NodeType.IF, condition['src']) condition_node.add_expression(condition) condition_node.analyze_expressions(self) - true_node = self._new_node(node.type) + true_node = self._new_node(node.type, true_expr['src']) if node.type == NodeType.VARIABLE: true_node.add_variable_declaration(node.variable_declaration) true_node.add_expression(true_expr) true_node.analyze_expressions(self) - false_node = self._new_node(node.type) + false_node = self._new_node(node.type, false_expr['src']) if node.type == NodeType.VARIABLE: false_node.add_variable_declaration(node.variable_declaration) false_node.add_expression(false_expr) false_node.analyze_expressions(self) - endif_node = self._new_node(NodeType.ENDIF) + endif_node = self._new_node(NodeType.ENDIF, condition['src']) for father in node.fathers: father.remove_son(node)