From e013338f812f2b2f946bea8455aec7976a3a6d4b Mon Sep 17 00:00:00 2001 From: Josselin Date: Wed, 24 Apr 2019 20:12:25 +0100 Subject: [PATCH 01/11] Use crytic-compile from pip --- scripts/travis_install.sh | 5 ----- setup.py | 2 +- slither/slithir/variables/state_variable.py | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/scripts/travis_install.sh b/scripts/travis_install.sh index fed178c60..637f3f748 100755 --- a/scripts/travis_install.sh +++ b/scripts/travis_install.sh @@ -16,8 +16,3 @@ function install_solc { install_solc - -git clone https://github.com/crytic/crytic-compile -cd crytic-compile -pip install . - diff --git a/setup.py b/setup.py index 525839e69..3e8902de6 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ setup( version='0.6.2', packages=find_packages(), python_requires='>=3.6', - install_requires=['prettytable>=0.7.2', 'pysha3>=1.0.2'], + install_requires=['prettytable>=0.7.2', 'pysha3>=1.0.2', 'crytic-compile>=0.1.0'], license='AGPL-3.0', long_description=open('README.md').read(), entry_points={ diff --git a/slither/slithir/variables/state_variable.py b/slither/slithir/variables/state_variable.py index ecefe98c7..d2f5d1d6b 100644 --- a/slither/slithir/variables/state_variable.py +++ b/slither/slithir/variables/state_variable.py @@ -43,4 +43,4 @@ class StateIRVariable(StateVariable, SlithIRVariable): @property def ssa_name(self): - return '{}_{}'.format(self._name, self.index) + return '{}.{}_{}'.format(self.id, self._name, self.index) From 3991f6e4941c88996639148195ce14e712fb85b0 Mon Sep 17 00:00:00 2001 From: Josselin Date: Fri, 26 Apr 2019 13:24:04 +0100 Subject: [PATCH 02/11] Breaking change: functions and modifiers are not shared accross contracts, each contract has an own copy of all the functions/modifiers Changed: - function/modifier inherits from ChildInheritance, which contains original_contract, pointing to the base contract - function.contract will point to the contract where the instance is stored - function.is_shadowed indicates if a function is shadowed - function.canonical_name returns original_contract+ '.'+ name - contract._functions/_modifiers are indexes by their canonical_name - contract.functions_as_dict is rename available_functions_as_dict and return non-shadowed functions (same for modifiers_as_dict) - For better clarity, all the detectors use now canoncal_name, for variable/function/structure/... rather than contract.name + name - abstract_detector.add_event_to_json/ abstract_detector.add_events_to_json (the json type is still 'function' to simplify the 3-party parsing) --- examples/scripts/functions_called.py | 2 +- examples/scripts/possible_paths.py | 4 +- examples/scripts/slithIR.py | 2 +- slither/core/children/child_contract.py | 2 + slither/core/children/child_inheritance.py | 13 +++ slither/core/declarations/contract.py | 44 ++++++++-- slither/core/declarations/event.py | 8 ++ slither/core/declarations/function.py | 26 +++++- slither/core/variables/local_variable.py | 5 ++ slither/detectors/abstract_detector.py | 17 +++- .../detectors/attributes/const_functions.py | 13 ++- .../detectors/erc20/incorrect_interface.py | 2 +- .../erc20/unindexed_event_parameters.py | 2 +- slither/detectors/functions/arbitrary_send.py | 7 +- .../detectors/functions/complex_function.py | 5 +- .../detectors/functions/external_function.py | 5 +- slither/detectors/functions/suicidal.py | 7 +- .../naming_convention/naming_convention.py | 38 ++++---- .../detectors/operations/block_timestamp.py | 7 +- .../detectors/operations/low_level_calls.py | 6 +- .../operations/unused_return_values.py | 7 +- .../detectors/reentrancy/reentrancy_benign.py | 4 +- .../detectors/reentrancy/reentrancy_eth.py | 4 +- .../reentrancy_read_before_write.py | 4 +- .../detectors/shadowing/builtin_symbols.py | 8 +- slither/detectors/shadowing/local.py | 11 +-- slither/detectors/shadowing/state.py | 10 +-- slither/detectors/statements/assembly.py | 6 +- slither/detectors/statements/calls_in_loop.py | 6 +- .../statements/controlled_delegatecall.py | 2 +- .../statements/incorrect_strict_equality.py | 5 +- slither/detectors/statements/tx_origin.py | 4 +- .../possible_const_state_variables.py | 5 +- .../uninitialized_local_variables.py | 7 +- .../uninitialized_state_variables.py | 5 +- .../uninitialized_storage_variables.py | 4 +- .../variables/unused_state_variables.py | 7 +- .../printers/inheritance/inheritance_graph.py | 6 +- slither/printers/summary/slithir.py | 40 ++++----- slither/printers/summary/slithir_ssa.py | 36 ++++---- slither/slithir/operations/internal_call.py | 5 +- slither/slithir/variables/state_variable.py | 2 +- slither/solc_parsing/declarations/contract.py | 88 +++++++++++++------ slither/solc_parsing/declarations/function.py | 3 +- .../expressions/expression_parsing.py | 32 ++++--- .../solidity_types/type_parsing.py | 6 +- slither/utils/inheritance_analysis.py | 2 +- utils/possible_paths/__main__.py | 6 +- utils/upgradeability/check_initialization.py | 4 +- 49 files changed, 322 insertions(+), 222 deletions(-) create mode 100644 slither/core/children/child_inheritance.py diff --git a/examples/scripts/functions_called.py b/examples/scripts/functions_called.py index 5f25477d0..4324ac902 100644 --- a/examples/scripts/functions_called.py +++ b/examples/scripts/functions_called.py @@ -16,7 +16,7 @@ entry_point = contract.get_function_from_signature('entry_point()') all_calls = entry_point.all_internal_calls() -all_calls_formated = [f.contract.name + '.' + f.name for f in all_calls] +all_calls_formated = [f.canonical_name for f in all_calls] # Print the result print('From entry_point the functions reached are {}'.format(all_calls_formated)) diff --git a/examples/scripts/possible_paths.py b/examples/scripts/possible_paths.py index e65ddb1c8..6c3b6d0be 100644 --- a/examples/scripts/possible_paths.py +++ b/examples/scripts/possible_paths.py @@ -179,12 +179,12 @@ reaching_functions = set([y for x in reaching_paths for y in x if y not in targe # Print out all function names which can reach the targets. print(f"The following functions reach the specified targets:") -for function_desc in sorted([f"{f.contract.name}.{f.full_name}" for f in reaching_functions]): +for function_desc in sorted([f"{f.canonical_name}" for f in reaching_functions]): print(f"-{function_desc}") print("\n") # Format all function paths. -reaching_paths_str = [' -> '.join([f"{f.contract.name}.{f.full_name}" for f in reaching_path]) for reaching_path in reaching_paths] +reaching_paths_str = [' -> '.join([f"{f.canonical_name}" for f in reaching_path]) for reaching_path in reaching_paths] # Print a sorted list of all function paths which can reach the targets. print(f"The following paths reach the specified targets:") diff --git a/examples/scripts/slithIR.py b/examples/scripts/slithIR.py index 04fe255c8..b58e06f10 100644 --- a/examples/scripts/slithIR.py +++ b/examples/scripts/slithIR.py @@ -15,7 +15,7 @@ for contract in slither.contracts: for function in contract.functions: # Dont explore inherited functions - if function.contract == contract: + if function.original_contract == contract: print('Function: {}'.format(function.name)) diff --git a/slither/core/children/child_contract.py b/slither/core/children/child_contract.py index d5a613bc9..6e476d59f 100644 --- a/slither/core/children/child_contract.py +++ b/slither/core/children/child_contract.py @@ -4,6 +4,7 @@ class ChildContract: def __init__(self): super(ChildContract, self).__init__() self._contract = None + self._original_contract = None def set_contract(self, contract): self._contract = contract @@ -11,3 +12,4 @@ class ChildContract: @property def contract(self): return self._contract + diff --git a/slither/core/children/child_inheritance.py b/slither/core/children/child_inheritance.py new file mode 100644 index 000000000..668c37a5f --- /dev/null +++ b/slither/core/children/child_inheritance.py @@ -0,0 +1,13 @@ + +class ChildInheritance: + + def __init__(self): + super(ChildInheritance, self).__init__() + self._original_contract = None + + def set_original_contract(self, original_contract): + self._original_contract = original_contract + + @property + def original_contract(self): + return self._original_contract diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 787172843..5f1c0a2e9 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -32,6 +32,7 @@ class Contract(ChildSlither, SourceMapping): self._variables = {} self._modifiers = {} self._functions = {} + self._using_for = {} self._kind = None @@ -183,7 +184,7 @@ class Contract(ChildSlither, SourceMapping): @property def constructor_not_inherited(self): - return next((func for func in self.functions if func.is_constructor and func.contract == self), None) + return next((func for func in self.functions if func.is_constructor and func.original_contract == self), None) @property def constructors(self): @@ -219,22 +220,22 @@ class Contract(ChildSlither, SourceMapping): ''' return list(self._functions.values()) - def functions_as_dict(self): - return self._functions + def available_functions_as_dict(self): + return {f.full_name: f for f in self._functions.values() if not f.is_shadowed} @property def functions_inherited(self): ''' list(Function): List of the inherited functions ''' - return [f for f in self.functions if f.contract != self] + return [f for f in self.functions if f.original_contract != self] @property def functions_not_inherited(self): ''' list(Function): List of the functions defined within the contract (not inherited) ''' - return [f for f in self.functions if f.contract == self] + return [f for f in self.functions if f.original_contract == self] @property def functions_entry_points(self): @@ -250,22 +251,22 @@ class Contract(ChildSlither, SourceMapping): ''' return list(self._modifiers.values()) - def modifiers_as_dict(self): - return self._modifiers + def available_modifiers_as_dict(self): + return {m.full_name: m for m in self._modifiers.values() if not m.is_shadowed} @property def modifiers_inherited(self): ''' list(Modifier): List of the inherited modifiers ''' - return [m for m in self.modifiers if m.contract != self] + return [m for m in self.modifiers if m.original_contract != self] @property def modifiers_not_inherited(self): ''' list(Modifier): List of the modifiers defined within the contract (not inherited) ''' - return [m for m in self.modifiers if m.contract == self] + return [m for m in self.modifiers if m.original_contract == self] @property def functions_and_modifiers(self): @@ -288,6 +289,31 @@ class Contract(ChildSlither, SourceMapping): ''' return self.functions_not_inherited + self.modifiers_not_inherited + def available_elements_from_inheritances(self, elements, getter_available): + """ + + :param elements: dict(canonical_name -> elements) + :param getter_available: fun x + :return: + """ + # keep track of the contracts visited + # to prevent an ovveride due to multiple inheritance of the same contract + # A is B, C, D is C, --> the second C was already seen + inherited_elements = {} + accessible_elements = {} + contracts_visited = [] + for father in self.inheritance_reverse: + functions = {v.full_name: v for (_, v) in getter_available(father) + if not v.contract in contracts_visited} + contracts_visited.append(father) + inherited_elements.update(functions) + + for element in inherited_elements.values(): + accessible_elements[element.full_name] = elements[element.canonical_name] + + return accessible_elements + + # endregion ################################################################################### ################################################################################### diff --git a/slither/core/declarations/event.py b/slither/core/declarations/event.py index 0974c4773..29811f946 100644 --- a/slither/core/declarations/event.py +++ b/slither/core/declarations/event.py @@ -29,6 +29,14 @@ class Event(ChildContract, SourceMapping): name, parameters = self.signature return name+'('+','.join(parameters)+')' + @property + def canonical_name(self): + ''' Return the function signature as a str + Returns: + str: contract.func_name(type1,type2) + ''' + return self.contract.name + self.full_name + @property def elems(self): return self._elems diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index 5c8849880..27ffa6996 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -6,6 +6,7 @@ from collections import namedtuple from itertools import groupby from slither.core.children.child_contract import ChildContract +from slither.core.children.child_inheritance import ChildInheritance from slither.core.declarations.solidity_variables import (SolidityFunction, SolidityVariable, SolidityVariableComposed) @@ -18,7 +19,7 @@ logger = logging.getLogger("Function") ReacheableNode = namedtuple('ReacheableNode', ['node', 'ir']) -class Function(ChildContract, SourceMapping): +class Function(ChildContract, ChildInheritance, SourceMapping): """ Function class """ @@ -82,6 +83,8 @@ class Function(ChildContract, SourceMapping): self._all_conditional_solidity_variables_read_with_loop = None self._all_solidity_variables_used_as_args = None + self._is_shadowed = False + # set(ReacheableNode) self._reachable_from_nodes = set() self._reachable_from_functions = set() @@ -113,12 +116,21 @@ class Function(ChildContract, SourceMapping): name, parameters, _ = self.signature return name+'('+','.join(parameters)+')' + @property + def canonical_name(self): + """ + str: contract.func_name(type1,type2) + Return the function signature without the return values + """ + name, parameters, _ = self.signature + return self.original_contract.name + '.' + name + '(' + ','.join(parameters) + ')' + @property def is_constructor(self): """ bool: True if the function is the constructor """ - return self._is_constructor or self._name == self.contract.name + return self._is_constructor or self._name == self.original_contract.name @property def contains_assembly(self): @@ -170,6 +182,14 @@ class Function(ChildContract, SourceMapping): """ return self._pure + @property + def is_shadowed(self): + return self._is_shadowed + + @is_shadowed.setter + def is_shadowed(self, is_shadowed): + self._is_shadowed = is_shadowed + # endregion ################################################################################### ################################################################################### @@ -930,7 +950,7 @@ class Function(ChildContract, SourceMapping): (str, str, str, list(str), list(str), listr(str), list(str), list(str); contract_name, name, visibility, modifiers, vars read, vars written, internal_calls, external_calls_as_expressions """ - return (self.contract.name, self.full_name, self.visibility, + return (self.original_contract.name, self.full_name, self.visibility, [str(x) for x in self.modifiers], [str(x) for x in self.state_variables_read + self.solidity_variables_read], [str(x) for x in self.state_variables_written], diff --git a/slither/core/variables/local_variable.py b/slither/core/variables/local_variable.py index 414910f67..39e237271 100644 --- a/slither/core/variables/local_variable.py +++ b/slither/core/variables/local_variable.py @@ -50,3 +50,8 @@ class LocalVariable(ChildFunction, Variable): return False + @property + def canonical_name(self): + return self.name + + diff --git a/slither/detectors/abstract_detector.py b/slither/detectors/abstract_detector.py index 0763ea75c..37324f80a 100644 --- a/slither/detectors/abstract_detector.py +++ b/slither/detectors/abstract_detector.py @@ -169,17 +169,32 @@ class AbstractDetector(metaclass=abc.ABCMeta): @staticmethod def add_function_to_json(function, d): contract = {'elements':[]} - AbstractDetector.add_contract_to_json(function.contract, contract) + AbstractDetector.add_contract_to_json(function.original_contract, contract) d['elements'].append({'type': 'function', 'name': function.name, 'source_mapping': function.source_mapping, 'contract': contract['elements'][0]}) + # We use the same json type for function and event to facilitate the third-party tools parsing + @staticmethod + def add_event_to_json(event, d): + contract = {'elements':[]} + AbstractDetector.add_contract_to_json(event.contract, contract) + d['elements'].append({'type': 'function', + 'name': event.name, + 'source_mapping': event.source_mapping, + 'contract': contract['elements'][0]}) + @staticmethod def add_functions_to_json(functions, d): for function in sorted(functions, key=lambda x: x.name): AbstractDetector.add_function_to_json(function, d) + @staticmethod + def add_events_to_json(events, d): + for event in sorted(events, key=lambda x: x.name): + AbstractDetector.add_event_to_json(event, d) + @staticmethod def add_nodes_to_json(nodes, d): for node in sorted(nodes, key=lambda x: x.node_id): diff --git a/slither/detectors/attributes/const_functions.py b/slither/detectors/attributes/const_functions.py index 029cfda8c..f38693f77 100644 --- a/slither/detectors/attributes/const_functions.py +++ b/slither/detectors/attributes/const_functions.py @@ -51,13 +51,13 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' results = [] for c in self.contracts: for f in c.functions: - if f.contract != c: + if f.original_contract != c: continue if f.view or f.pure: if f.contains_assembly: attr = 'view' if f.view else 'pure' - info = '{}.{} ({}) is declared {} but contains assembly code\n' - info = info.format(f.contract.name, f.name, f.source_mapping_str, attr) + info = '{} ({}) is declared {} but contains assembly code\n' + info = info.format(f.canonical_name, f.source_mapping_str, attr) json = self.generate_json_result(info) self.add_function_to_json(f, json) json['elements'].append({'type': 'info', @@ -67,11 +67,10 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' variables_written = f.all_state_variables_written() if variables_written: attr = 'view' if f.view else 'pure' - info = '{}.{} ({}) is declared {} but changes state variables:\n' - info = info.format(f.contract.name, f.name, f.source_mapping_str, attr) + info = '{} ({}) is declared {} but changes state variables:\n' + info = info.format(f.canonical_name, f.source_mapping_str, attr) for variable_written in variables_written: - info += '\t- {}.{}\n'.format(variable_written.contract.name, - variable_written.name) + info += '\t- {}\n'.format(variable_written.canonical_name) json = self.generate_json_result(info) diff --git a/slither/detectors/erc20/incorrect_interface.py b/slither/detectors/erc20/incorrect_interface.py index 94dc6f15b..dbd0feb92 100644 --- a/slither/detectors/erc20/incorrect_interface.py +++ b/slither/detectors/erc20/incorrect_interface.py @@ -52,7 +52,7 @@ contract Token{ Returns: list(str) : list of incorrect function signatures """ - functions = [f for f in contract.functions if f.contract == contract and \ + functions = [f for f in contract.functions if f.original_contract == contract and \ IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] return functions diff --git a/slither/detectors/erc20/unindexed_event_parameters.py b/slither/detectors/erc20/unindexed_event_parameters.py index c64a7681a..71327f94e 100644 --- a/slither/detectors/erc20/unindexed_event_parameters.py +++ b/slither/detectors/erc20/unindexed_event_parameters.py @@ -79,7 +79,7 @@ In this case, Transfer and Approval events should have the 'indexed' keyword on # Add the events to the JSON (note: we do not add the params/vars as they have no source mapping). json = self.generate_json_result(info) - self.add_functions_to_json([event for event, _ in unindexed_params], json) + self.add_events_to_json([event for event, _ in unindexed_params], json) results.append(json) return results diff --git a/slither/detectors/functions/arbitrary_send.py b/slither/detectors/functions/arbitrary_send.py index 59f33fcab..e5900a3c5 100644 --- a/slither/detectors/functions/arbitrary_send.py +++ b/slither/detectors/functions/arbitrary_send.py @@ -94,7 +94,7 @@ Bob calls `setDestination` and `withdraw`. As a result he withdraws the contract list((Function), (list (Node))) """ ret = [] - for f in [f for f in contract.functions if f.contract == contract]: + for f in [f for f in contract.functions if f.original_contract == contract]: nodes = self.arbitrary_send(f) if nodes: ret.append((f, nodes)) @@ -109,9 +109,8 @@ Bob calls `setDestination` and `withdraw`. As a result he withdraws the contract arbitrary_send = self.detect_arbitrary_send(c) for (func, nodes) in arbitrary_send: - info = "{}.{} ({}) sends eth to arbitrary user\n" - info = info.format(func.contract.name, - func.name, + info = "{} ({}) sends eth to arbitrary user\n" + info = info.format(func.canonical_name, func.source_mapping_str) info += '\tDangerous calls:\n' for node in nodes: diff --git a/slither/detectors/functions/complex_function.py b/slither/detectors/functions/complex_function.py index 4f0c9093a..2e3c0ac26 100644 --- a/slither/detectors/functions/complex_function.py +++ b/slither/detectors/functions/complex_function.py @@ -90,7 +90,7 @@ class ComplexFunction(AbstractDetector): for issue in issues: func, cause = issue.values() - txt = "{}.{} ({}) is a complex function:\n" + txt = "{} ({}) is a complex function:\n" if cause == self.CAUSE_EXTERNAL_CALL: txt += "\t- Reason: High number of external calls" @@ -99,8 +99,7 @@ class ComplexFunction(AbstractDetector): if cause == self.CAUSE_STATE_VARS: txt += "\t- Reason: High number of modified state variables" - info = txt.format(func.contract.name, - func.name, + info = txt.format(func.canonical_name, func.source_mapping_str) info = info + "\n" self.log(info) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 1d80e4e95..50e0d52b2 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -165,9 +165,8 @@ class ExternalFunction(AbstractDetector): # Loop for each function definition, and recommend it be declared external. for function_definition in all_function_definitions: - txt = "{}.{} ({}) should be declared external\n" - info = txt.format(function_definition.contract.name, - function_definition.name, + txt = "{} ({}) should be declared external\n" + info = txt.format(function_definition.canonical_name, function_definition.source_mapping_str) json = self.generate_json_result(info) diff --git a/slither/detectors/functions/suicidal.py b/slither/detectors/functions/suicidal.py index 66751dc9c..f20c4da13 100644 --- a/slither/detectors/functions/suicidal.py +++ b/slither/detectors/functions/suicidal.py @@ -59,7 +59,7 @@ Bob calls `kill` and destructs the contract.''' def detect_suicidal(self, contract): ret = [] - for f in [f for f in contract.functions if f.contract == contract]: + for f in [f for f in contract.functions if f.original_contract == contract]: if self.detect_suicidal_func(f): ret.append(f) return ret @@ -72,9 +72,8 @@ Bob calls `kill` and destructs the contract.''' functions = self.detect_suicidal(c) for func in functions: - txt = "{}.{} ({}) allows anyone to destruct the contract\n" - info = txt.format(func.contract.name, - func.name, + txt = "{} ({}) allows anyone to destruct the contract\n" + info = txt.format(func.canonical_name, func.source_mapping_str) json = self.generate_json_result(info) diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index 545d14e06..4ec7fc6d1 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -90,8 +90,8 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 continue if not self.is_cap_words(event.name): - info = "Event '{}.{}' ({}) is not in CapWords\n" - info = info.format(event.contract.name, event.name, event.source_mapping_str) + info = "Event '{}' ({}) is not in CapWords\n" + info = info.format(event.canonical_name, event.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -103,12 +103,12 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 results.append(json) for func in contract.functions: - if func.contract != contract: + if func.original_contract != contract: continue if not self.is_mixed_case(func.name): - info = "Function '{}.{}' ({}) is not in mixedCase\n" - info = info.format(func.contract.name, func.name, func.source_mapping_str) + info = "Function '{}' ({}) is not in mixedCase\n" + info = info.format(func.canonical_name, func.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -125,10 +125,9 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 else: correct_naming = self.is_mixed_case_with_underscore(argument.name) if not correct_naming: - info = "Parameter '{}' of {}.{} ({}) is not in mixedCase\n" + info = "Parameter '{}' of {} ({}) is not in mixedCase\n" info = info.format(argument.name, - argument.function.contract.name, - argument.function, + argument.canonical_name, argument.source_mapping_str) json = self.generate_json_result(info) @@ -146,8 +145,8 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 if self.should_avoid_name(var.name): if not self.is_upper_case_with_underscores(var.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) + info = "Variable '{}' ({}) used l, O, I, which should not be used\n" + info = info.format(var.canonical_name, var.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -164,8 +163,8 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 continue if not self.is_upper_case_with_underscores(var.name): - info = "Constant '{}.{}' ({}) is not in UPPER_CASE_WITH_UNDERSCORES\n" - info = info.format(var.contract.name, var.name, var.source_mapping_str) + info = "Constant '{}' ({}) is not in UPPER_CASE_WITH_UNDERSCORES\n" + info = info.format(var.canonical_name, var.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -182,8 +181,8 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 else: correct_naming = self.is_mixed_case(var.name) if not correct_naming: - info = "Variable '{}.{}' ({}) is not in mixedCase\n" - info = info.format(var.contract.name, var.name, var.source_mapping_str) + info = "Variable '{}' ({}) is not in mixedCase\n" + info = info.format(var.canonical_name, var.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -199,8 +198,8 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 continue if not self.is_cap_words(enum.name): - info = "Enum '{}.{}' ({}) is not in CapWords\n" - info = info.format(enum.contract.name, enum.name, enum.source_mapping_str) + info = "Enum '{}' ({}) is not in CapWords\n" + info = info.format(enum.canonical_name, enum.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -213,13 +212,12 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 for modifier in contract.modifiers: - if modifier.contract != contract: + if modifier.original_contract != contract: continue if not self.is_mixed_case(modifier.name): - info = "Modifier '{}.{}' ({}) is not in mixedCase\n" - info = info.format(modifier.contract.name, - modifier.name, + info = "Modifier '{}' ({}) is not in mixedCase\n" + info = info.format(modifier.canonical_name, modifier.source_mapping_str) json = self.generate_json_result(info) diff --git a/slither/detectors/operations/block_timestamp.py b/slither/detectors/operations/block_timestamp.py index 86912098d..ecadb2e94 100644 --- a/slither/detectors/operations/block_timestamp.py +++ b/slither/detectors/operations/block_timestamp.py @@ -54,7 +54,7 @@ class Timestamp(AbstractDetector): list((Function), (list (Node))) """ ret = [] - for f in [f for f in contract.functions if f.contract == contract]: + for f in [f for f in contract.functions if f.original_contract == contract]: nodes = self.timestamp(f) if nodes: ret.append((f, nodes)) @@ -69,9 +69,8 @@ class Timestamp(AbstractDetector): dangerous_timestamp = self.detect_dangerous_timestamp(c) for (func, nodes) in dangerous_timestamp: - info = "{}.{} ({}) uses timestamp for comparisons\n" - info = info.format(func.contract.name, - func.name, + info = "{} ({}) uses timestamp for comparisons\n" + info = info.format(func.canonical_name, func.source_mapping_str) info += '\tDangerous comparisons:\n' for node in nodes: diff --git a/slither/detectors/operations/low_level_calls.py b/slither/detectors/operations/low_level_calls.py index 9123a2c67..9a819518d 100644 --- a/slither/detectors/operations/low_level_calls.py +++ b/slither/detectors/operations/low_level_calls.py @@ -33,7 +33,7 @@ class LowLevelCalls(AbstractDetector): def detect_low_level_calls(self, contract): ret = [] - for f in [f for f in contract.functions if contract == f.contract]: + for f in [f for f in contract.functions if contract == f.original_contract]: nodes = f.nodes assembly_nodes = [n for n in nodes if self._contains_low_level_calls(n)] @@ -48,8 +48,8 @@ class LowLevelCalls(AbstractDetector): for c in self.contracts: values = self.detect_low_level_calls(c) for func, nodes in values: - info = "Low level call in {}.{} ({}):\n" - info = info.format(func.contract.name, func.name, func.source_mapping_str) + info = "Low level call in {} ({}):\n" + info = info.format(func.canonical_name, func.source_mapping_str) for node in nodes: info += "\t-{} {}\n".format(str(node.expression), node.source_mapping_str) diff --git a/slither/detectors/operations/unused_return_values.py b/slither/detectors/operations/unused_return_values.py index 04e50398b..ee153f03a 100644 --- a/slither/detectors/operations/unused_return_values.py +++ b/slither/detectors/operations/unused_return_values.py @@ -64,13 +64,12 @@ contract MyConc{ results = [] for c in self.slither.contracts: for f in c.functions + c.modifiers: - if f.contract != c: + if f.original_contract != c: continue unused_return = self.detect_unused_return_values(f) if unused_return: - info = "{}.{} ({}) does not use the value returned by external calls:\n" - info = info.format(f.contract.name, - f.name, + info = "{} ({}) does not use the value returned by external calls:\n" + info = info.format(f.canonical_name, f.source_mapping_str) for node in unused_return: info += "\t-{} ({})\n".format(node.expression, node.source_mapping_str) diff --git a/slither/detectors/reentrancy/reentrancy_benign.py b/slither/detectors/reentrancy/reentrancy_benign.py index a2d570e60..7cecc3e64 100644 --- a/slither/detectors/reentrancy/reentrancy_benign.py +++ b/slither/detectors/reentrancy/reentrancy_benign.py @@ -84,8 +84,8 @@ Only report reentrancy that acts as a double call (see `reentrancy-eth`, `reentr for (func, calls, send_eth), varsWritten in result_sorted: calls = sorted(list(set(calls)), key=lambda x: x.node_id) send_eth = sorted(list(set(send_eth)), key=lambda x: x.node_id) - info = 'Reentrancy in {}.{} ({}):\n' - info = info.format(func.contract.name, func.name, func.source_mapping_str) + info = 'Reentrancy in {} ({}):\n' + info = info.format(func.canonical_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) diff --git a/slither/detectors/reentrancy/reentrancy_eth.py b/slither/detectors/reentrancy/reentrancy_eth.py index 8d0691a5b..0ea1b9357 100644 --- a/slither/detectors/reentrancy/reentrancy_eth.py +++ b/slither/detectors/reentrancy/reentrancy_eth.py @@ -87,8 +87,8 @@ Bob uses the re-entrancy bug to call `withdrawBalance` two times, and withdraw m calls = sorted(list(set(calls)), key=lambda x: x.node_id) send_eth = sorted(list(set(send_eth)), key=lambda x: x.node_id) - info = 'Reentrancy in {}.{} ({}):\n' - info = info.format(func.contract.name, func.name, func.source_mapping_str) + info = 'Reentrancy in {} ({}):\n' + info = info.format(func.canonical_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) diff --git a/slither/detectors/reentrancy/reentrancy_read_before_write.py b/slither/detectors/reentrancy/reentrancy_read_before_write.py index 379585285..95c0e5012 100644 --- a/slither/detectors/reentrancy/reentrancy_read_before_write.py +++ b/slither/detectors/reentrancy/reentrancy_read_before_write.py @@ -82,8 +82,8 @@ Do not report reentrancies that involve ethers (see `reentrancy-eth`)''' result_sorted = sorted(list(reentrancies.items()), key=lambda x:x[0][0].name) for (func, calls), varsWritten in result_sorted: calls = sorted(list(set(calls)), key=lambda x: x.node_id) - info = 'Reentrancy in {}.{} ({}):\n' - info = info.format(func.contract.name, func.name, func.source_mapping_str) + info = 'Reentrancy in {} ({}):\n' + info = info.format(func.canonical_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) diff --git a/slither/detectors/shadowing/builtin_symbols.py b/slither/detectors/shadowing/builtin_symbols.py index dc9981c41..cc8a1f19e 100644 --- a/slither/detectors/shadowing/builtin_symbols.py +++ b/slither/detectors/shadowing/builtin_symbols.py @@ -91,12 +91,12 @@ contract Bug { # Loop through all functions, modifiers, variables (state and local) to detect any built-in symbol keywords. for function in contract.functions: - if function.contract == contract: + if function.original_contract == contract: if self.is_builtin_symbol(function.name): result.append((self.SHADOWING_FUNCTION, function, None)) result += self.detect_builtin_shadowing_locals(function) for modifier in contract.modifiers: - if modifier.contract == contract: + if modifier.original_contract == contract: if self.is_builtin_symbol(modifier.name): result.append((self.SHADOWING_MODIFIER, modifier, None)) result += self.detect_builtin_shadowing_locals(modifier) @@ -143,8 +143,10 @@ contract Bug { # Generate relevant JSON data for this shadowing definition. json = self.generate_json_result(info) - if shadow_type in [self.SHADOWING_FUNCTION, self.SHADOWING_MODIFIER, self.SHADOWING_EVENT]: + if shadow_type in [self.SHADOWING_FUNCTION, self.SHADOWING_MODIFIER]: self.add_function_to_json(shadow_object, json) + elif shadow_type == self.SHADOWING_EVENT: + self.add_event_to_json(shadow_object, json) elif shadow_type in [self.SHADOWING_STATE_VARIABLE, self.SHADOWING_LOCAL_VARIABLE]: self.add_variable_to_json(shadow_object, json) results.append(json) diff --git a/slither/detectors/shadowing/local.py b/slither/detectors/shadowing/local.py index 0a04ebed7..20d5cebc7 100644 --- a/slither/detectors/shadowing/local.py +++ b/slither/detectors/shadowing/local.py @@ -59,7 +59,7 @@ contract Bug { # Loop through all functions + modifiers in this contract. for function in contract.functions + contract.modifiers: # We should only look for functions declared directly in this contract (not in a base contract). - if function.contract != contract: + if function.original_contract != contract: continue # This function was declared in this contract, we check what its local variables might shadow. @@ -68,11 +68,11 @@ contract Bug { for scope_contract in [contract] + contract.inheritance: # Check functions for scope_function in scope_contract.functions: - if variable.name == scope_function.name and scope_function.contract == scope_contract: + if variable.name == scope_function.name and scope_function.original_contract == scope_contract: overshadowed.append((self.OVERSHADOWED_FUNCTION, scope_contract.name, scope_function)) # Check modifiers for scope_modifier in scope_contract.modifiers: - if variable.name == scope_modifier.name and scope_modifier.contract == scope_contract: + if variable.name == scope_modifier.name and scope_modifier.original_contract == scope_contract: overshadowed.append((self.OVERSHADOWED_MODIFIER, scope_contract.name, scope_modifier)) # Check events for scope_event in scope_contract.events: @@ -121,9 +121,10 @@ contract Bug { json = self.generate_json_result(info) self.add_variable_to_json(local_variable, json) for overshadowed_entry in overshadowed: - if overshadowed_entry[0] in [self.OVERSHADOWED_FUNCTION, self.OVERSHADOWED_MODIFIER, - self.OVERSHADOWED_EVENT]: + if overshadowed_entry[0] in [self.OVERSHADOWED_FUNCTION, self.OVERSHADOWED_MODIFIER]: self.add_function_to_json(overshadowed_entry[2], json) + elif overshadowed_entry[0] == self.OVERSHADOWED_EVENT: + self.add_event_to_json(overshadowed_entry[2], json) elif overshadowed_entry[0] == self.OVERSHADOWED_STATE_VARIABLE: self.add_variable_to_json(overshadowed_entry[2], json) results.append(json) diff --git a/slither/detectors/shadowing/state.py b/slither/detectors/shadowing/state.py index bd0b67d89..75ba66132 100644 --- a/slither/detectors/shadowing/state.py +++ b/slither/detectors/shadowing/state.py @@ -76,13 +76,11 @@ contract DerivedContract is BaseContract{ for all_variables in shadowing: shadow = all_variables[0] variables = all_variables[1:] - info = '{}.{} ({}) shadows:\n'.format(shadow.contract.name, - shadow.name, - shadow.source_mapping_str) + info = '{} ({}) shadows:\n'.format(shadow.canonical_name, + shadow.source_mapping_str) for var in variables: - info += "\t- {}.{} ({})\n".format(var.contract.name, - var.name, - var.source_mapping_str) + info += "\t- {} ({})\n".format(var.canonical_name, + var.source_mapping_str) json = self.generate_json_result(info) self.add_variables_to_json(all_variables, json) diff --git a/slither/detectors/statements/assembly.py b/slither/detectors/statements/assembly.py index e1b35a6f2..d95a81731 100644 --- a/slither/detectors/statements/assembly.py +++ b/slither/detectors/statements/assembly.py @@ -35,7 +35,7 @@ class Assembly(AbstractDetector): def detect_assembly(self, contract): ret = [] for f in contract.functions: - if f.contract != contract: + if f.original_contract != contract: continue nodes = f.nodes assembly_nodes = [n for n in nodes if @@ -51,8 +51,8 @@ class Assembly(AbstractDetector): for c in self.contracts: values = self.detect_assembly(c) for func, nodes in values: - info = "{}.{} uses assembly ({})\n" - info = info.format(func.contract.name, func.name, func.source_mapping_str) + info = "{} uses assembly ({})\n" + info = info.format(func.canonical_name, func.source_mapping_str) for node in nodes: info += "\t- {}\n".format(node.source_mapping_str) diff --git a/slither/detectors/statements/calls_in_loop.py b/slither/detectors/statements/calls_in_loop.py index 807ea7e23..b3fa39f26 100644 --- a/slither/detectors/statements/calls_in_loop.py +++ b/slither/detectors/statements/calls_in_loop.py @@ -72,7 +72,7 @@ If one of the destinations has a fallback function which reverts, `bad` will alw def detect_call_in_loop(contract): ret = [] for f in contract.functions + contract.modifiers: - if f.contract == contract and f.is_implemented: + if f.original_contract == contract and f.is_implemented: MultipleCallsInLoop.call_in_loop(f.entry_point, False, [], ret) @@ -86,8 +86,8 @@ If one of the destinations has a fallback function which reverts, `bad` will alw values = self.detect_call_in_loop(c) for node in values: func = node.function - info = "{}.{} has external calls inside a loop:\n" - info = info.format(func.contract.name, func.name) + info = "{} has external calls inside a loop:\n" + info = info.format(func.canonical_name) info += "\t- {} ({})\n".format(node.expression, node.source_mapping_str) diff --git a/slither/detectors/statements/controlled_delegatecall.py b/slither/detectors/statements/controlled_delegatecall.py index fbe657f32..0f721426e 100644 --- a/slither/detectors/statements/controlled_delegatecall.py +++ b/slither/detectors/statements/controlled_delegatecall.py @@ -42,7 +42,7 @@ Bob calls `delegate` and delegates the execution to its malicious contract. As a for contract in self.slither.contracts: for f in contract.functions: - if f.contract != contract: + if f.original_contract != contract: continue nodes = self.controlled_delegatecall(f) if nodes: diff --git a/slither/detectors/statements/incorrect_strict_equality.py b/slither/detectors/statements/incorrect_strict_equality.py index a1fedf140..05f2aa42a 100644 --- a/slither/detectors/statements/incorrect_strict_equality.py +++ b/slither/detectors/statements/incorrect_strict_equality.py @@ -112,9 +112,8 @@ contract Crowdsale{ # sort ret to get deterministic results ret = sorted(list(ret.items()), key=lambda x:x[0].name) for f, nodes in ret: - info += "{}.{} ({}) uses a dangerous strict equality:\n".format(f.contract.name, - f.name, - f.source_mapping_str) + info += "{} ({}) uses a dangerous strict equality:\n".format(f.canonical_name, + f.source_mapping_str) # sort the nodes to get deterministic results nodes.sort(key=lambda x: x.node_id) diff --git a/slither/detectors/statements/tx_origin.py b/slither/detectors/statements/tx_origin.py index bbd86d9bf..995ac53d6 100644 --- a/slither/detectors/statements/tx_origin.py +++ b/slither/detectors/statements/tx_origin.py @@ -65,8 +65,8 @@ Bob is the owner of `TxOrigin`. Bob calls Eve's contract. Eve's contract calls ` for c in self.contracts: values = self.detect_tx_origin(c) for func, nodes in values: - info = "{}.{} uses tx.origin for authorization:\n" - info = info.format(func.contract.name, func.name) + info = "{} uses tx.origin for authorization:\n" + info = info.format(func.canonical_name) for node in nodes: info += "\t- {} ({})\n".format(node.expression, node.source_mapping_str) diff --git a/slither/detectors/variables/possible_const_state_variables.py b/slither/detectors/variables/possible_const_state_variables.py index 7e726be46..58c5cb66a 100644 --- a/slither/detectors/variables/possible_const_state_variables.py +++ b/slither/detectors/variables/possible_const_state_variables.py @@ -85,9 +85,8 @@ class ConstCandidateStateVars(AbstractDetector): # Order for deterministic results constable_variables = sorted(constable_variables, key=lambda x: x.canonical_name) for v in constable_variables: - info = "{}.{} should be constant ({})\n".format(v.contract.name, - v.name, - v.source_mapping_str) + info = "{} should be constant ({})\n".format(v.canonical_name, + v.source_mapping_str) all_info += info if all_info != '': json = self.generate_json_result(all_info) diff --git a/slither/detectors/variables/uninitialized_local_variables.py b/slither/detectors/variables/uninitialized_local_variables.py index d9854b339..289f844d5 100644 --- a/slither/detectors/variables/uninitialized_local_variables.py +++ b/slither/detectors/variables/uninitialized_local_variables.py @@ -90,7 +90,7 @@ Bob calls `transfer`. As a result, the ethers are sent to the address 0x0 and ar for contract in self.slither.contracts: for function in contract.functions: - if function.is_implemented and function.contract == contract: + if function.is_implemented and function.original_contract == contract: if function.contains_assembly: continue # dont consider storage variable, as they are detected by another detector @@ -101,10 +101,9 @@ Bob calls `transfer`. As a result, the ethers are sent to the address 0x0 and ar for(function, uninitialized_local_variable) in all_results: var_name = uninitialized_local_variable.name - info = "{} in {}.{} ({}) is a local variable never initialiazed\n" + info = "{} in {} ({}) is a local variable never initialiazed\n" info = info.format(var_name, - function.contract.name, - function.name, + function.canonical_name, uninitialized_local_variable.source_mapping_str) diff --git a/slither/detectors/variables/uninitialized_state_variables.py b/slither/detectors/variables/uninitialized_state_variables.py index 2dec5e7e5..18b89324b 100644 --- a/slither/detectors/variables/uninitialized_state_variables.py +++ b/slither/detectors/variables/uninitialized_state_variables.py @@ -92,9 +92,8 @@ Initialize all the variables. If a variable is meant to be initialized to zero, for c in self.slither.contracts_derived: ret = self.detect_uninitialized(c) for variable, functions in ret: - info = "{}.{} ({}) is never initialized. It is used in:\n" - info = info.format(variable.contract.name, - variable.name, + info = "{} ({}) is never initialized. It is used in:\n" + info = info.format(variable.canonical_name, variable.source_mapping_str) for f in functions: info += "\t- {} ({})\n".format(f.name, f.source_mapping_str) diff --git a/slither/detectors/variables/uninitialized_storage_variables.py b/slither/detectors/variables/uninitialized_storage_variables.py index 3cc5a56ef..ef1d9ba3c 100644 --- a/slither/detectors/variables/uninitialized_storage_variables.py +++ b/slither/detectors/variables/uninitialized_storage_variables.py @@ -105,8 +105,8 @@ Bob calls `func`. As a result, `owner` is override to 0. for(function, uninitialized_storage_variable) in self.results: var_name = uninitialized_storage_variable.name - info = "{} in {}.{} ({}) is a storage variable never initialiazed\n" - info = info.format(var_name, function.contract.name, function.name, uninitialized_storage_variable.source_mapping_str) + info = "{} in {} ({}) is a storage variable never initialiazed\n" + info = info.format(var_name, function.canonical_name, uninitialized_storage_variable.source_mapping_str) json = self.generate_json_result(info) diff --git a/slither/detectors/variables/unused_state_variables.py b/slither/detectors/variables/unused_state_variables.py index c713c33a9..b293ecb69 100644 --- a/slither/detectors/variables/unused_state_variables.py +++ b/slither/detectors/variables/unused_state_variables.py @@ -57,10 +57,9 @@ class UnusedStateVars(AbstractDetector): if unusedVars: info = '' for var in unusedVars: - info += "{}.{} ({}) is never used in {}\n".format(var.contract.name, - var.name, - var.source_mapping_str, - c.name) + info += "{} ({}) is never used in {}\n".format(var.canonical_name, + var.source_mapping_str, + c.name) json = self.generate_json_result(info) diff --git a/slither/printers/inheritance/inheritance_graph.py b/slither/printers/inheritance/inheritance_graph.py index 468969002..fb6e7d834 100644 --- a/slither/printers/inheritance/inheritance_graph.py +++ b/slither/printers/inheritance/inheritance_graph.py @@ -116,14 +116,14 @@ class PrinterInheritanceGraph(AbstractPrinter): # Functions visibilities = ['public', 'external'] public_functions = [self._get_pattern_func(f, contract) for f in contract.functions if - not f.is_constructor and f.contract == contract and f.visibility in visibilities] + not f.is_constructor and f.original_contract == contract and f.visibility in visibilities] public_functions = ''.join(public_functions) private_functions = [self._get_pattern_func(f, contract) for f in contract.functions if - not f.is_constructor and f.contract == contract and f.visibility not in visibilities] + not f.is_constructor and f.original_contract == contract and f.visibility not in visibilities] private_functions = ''.join(private_functions) # Modifiers - modifiers = [self._get_pattern_func(m, contract) for m in contract.modifiers if m.contract == contract] + modifiers = [self._get_pattern_func(m, contract) for m in contract.modifiers if m.original_contract == contract] modifiers = ''.join(modifiers) # Public variables diff --git a/slither/printers/summary/slithir.py b/slither/printers/summary/slithir.py index 3cad7b4e4..e34c9d34d 100644 --- a/slither/printers/summary/slithir.py +++ b/slither/printers/summary/slithir.py @@ -23,26 +23,24 @@ class PrinterSlithIR(AbstractPrinter): for contract in self.contracts: print('Contract {}'.format(contract.name)) for function in contract.functions: - if function.contract == contract: - print('\tFunction {}'.format(function.full_name)) - for node in function.nodes: - if node.expression: - print('\t\tExpression: {}'.format(node.expression)) - print('\t\tIRs:') - for ir in node.irs: - print('\t\t\t{}'.format(ir)) - elif node.irs: - print('\t\tIRs:') - for ir in node.irs: - print('\t\t\t{}'.format(ir)) + print('\tFunction {}'.format(function.canonical_name)) + for node in function.nodes: + if node.expression: + print('\t\tExpression: {}'.format(node.expression)) + print('\t\tIRs:') + for ir in node.irs: + print('\t\t\t{}'.format(ir)) + elif node.irs: + print('\t\tIRs:') + for ir in node.irs: + print('\t\t\t{}'.format(ir)) for modifier in contract.modifiers: - if modifier.contract == contract: - print('\tModifier {}'.format(modifier.full_name)) - for node in modifier.nodes: - print(node) - if node.expression: - print('\t\tExpression: {}'.format(node.expression)) - print('\t\tIRs:') - for ir in node.irs: - print('\t\t\t{}'.format(ir)) + print('\tModifier {}'.format(modifier.canonical_name)) + for node in modifier.nodes: + print(node) + if node.expression: + print('\t\tExpression: {}'.format(node.expression)) + print('\t\tIRs:') + for ir in node.irs: + print('\t\t\t{}'.format(ir)) self.info(txt) diff --git a/slither/printers/summary/slithir_ssa.py b/slither/printers/summary/slithir_ssa.py index 6227c4167..c97a291fa 100644 --- a/slither/printers/summary/slithir_ssa.py +++ b/slither/printers/summary/slithir_ssa.py @@ -23,24 +23,22 @@ class PrinterSlithIRSSA(AbstractPrinter): for contract in self.contracts: print('Contract {}'.format(contract.name)) for function in contract.functions: - if function.contract == contract: - print('\tFunction {}'.format(function.full_name)) - for node in function.nodes: - if node.expression: - print('\t\tExpression: {}'.format(node.expression)) - if node.irs_ssa: - print('\t\tIRs:') - for ir in node.irs_ssa: - print('\t\t\t{}'.format(ir)) + print('\tFunction {}'.format(function.canonical_name)) + for node in function.nodes: + if node.expression: + print('\t\tExpression: {}'.format(node.expression)) + if node.irs_ssa: + print('\t\tIRs:') + for ir in node.irs_ssa: + print('\t\t\t{}'.format(ir)) for modifier in contract.modifiers: - if modifier.contract == contract: - print('\tModifier {}'.format(modifier.full_name)) - for node in modifier.nodes: - print(node) - if node.expression: - print('\t\tExpression: {}'.format(node.expression)) - if node.irs_ssa: - print('\t\tIRs:') - for ir in node.irs_ssa: - print('\t\t\t{}'.format(ir)) + print('\tModifier {}'.format(modifier.canonical_name)) + for node in modifier.nodes: + print(node) + if node.expression: + print('\t\tExpression: {}'.format(node.expression)) + if node.irs_ssa: + print('\t\tIRs:') + for ir in node.irs_ssa: + print('\t\t\t{}'.format(ir)) self.info(txt) diff --git a/slither/slithir/operations/internal_call.py b/slither/slithir/operations/internal_call.py index 5f2210e90..c0e5b54f5 100644 --- a/slither/slithir/operations/internal_call.py +++ b/slither/slithir/operations/internal_call.py @@ -56,9 +56,8 @@ class InternalCall(Call, OperationWithLValue): lvalue = '{}({}) = '.format(self.lvalue, ','.join(str(x) for x in self.lvalue.type)) else: lvalue = '{}({}) = '.format(self.lvalue, self.lvalue.type) - txt = '{}INTERNAL_CALL, {}.{}({})' + txt = '{}INTERNAL_CALL, {}({})' return txt.format(lvalue, - self.function.contract.name, - self.function.full_name, + self.function.canonical_name, ','.join(args)) diff --git a/slither/slithir/variables/state_variable.py b/slither/slithir/variables/state_variable.py index d2f5d1d6b..ecefe98c7 100644 --- a/slither/slithir/variables/state_variable.py +++ b/slither/slithir/variables/state_variable.py @@ -43,4 +43,4 @@ class StateIRVariable(StateVariable, SlithIRVariable): @property def ssa_name(self): - return '{}.{}_{}'.format(self.id, self._name, self.index) + return '{}_{}'.format(self._name, self.index) diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 953210bf5..500b67a9c 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -232,8 +232,9 @@ class ContractSolc04(Contract): def _parse_modifier(self, modifier): - modif = ModifierSolc(modifier, self) + modif = ModifierSolc(modifier, self, self) modif.set_contract(self) + modif.set_original_contract(self) modif.set_offset(modifier['src'], self.slither) self.slither.add_modifier(modif) self._modifiers_no_params.append(modif) @@ -247,7 +248,7 @@ class ContractSolc04(Contract): return def _parse_function(self, function): - func = FunctionSolc(function, self) + func = FunctionSolc(function, self, self) func.set_offset(function['src'], self.slither) self.slither.add_function(func) self._functions_no_params.append(func) @@ -281,26 +282,52 @@ class ContractSolc04(Contract): return def analyze_params_modifiers(self): - for father in self.inheritance_reverse: - self._modifiers.update(father.modifiers_as_dict()) - for modifier in self._modifiers_no_params: - modifier.analyze_params() - self._modifiers[modifier.full_name] = modifier + elements_no_params = self._modifiers_no_params + getter = lambda f: f.modifiers + getter_available = lambda f: f.available_modifiers_as_dict().items() + Cls = ModifierSolc + self._modifiers = self._analyze_params_elements(elements_no_params, getter, getter_available, Cls) self._modifiers_no_params = [] + return def analyze_params_functions(self): - # keep track of the contracts visited - # to prevent an ovveride due to multiple inheritance of the same contract - # A is B, C, D is C, --> the second C was already seen - contracts_visited = [] - for father in self.inheritance_reverse: - functions = {k:v for (k, v) in father.functions_as_dict().items() - if not v.contract in contracts_visited} - contracts_visited.append(father) - self._functions.update(functions) + + elements_no_params = self._functions_no_params + getter = lambda f: f.functions + getter_available = lambda f: f.available_functions_as_dict().items() + Cls = FunctionSolc + self._functions = self._analyze_params_elements(elements_no_params, getter, getter_available, Cls) + + self._functions_no_params = [] + return + + + def _analyze_params_elements(self, elements_no_params, getter, getter_available, Cls): + """ + Analyze the parameters of the given elements (Function or Modifier). + The function iterates over the inheritance to create an instance or inherited elements (Function or Modifier) + If the element is shadowed, set is_shadowed to True + :param elements_no_params: list of elements to analyzer + :param getter: fun x + :param getter_available: fun x + :param Cls: Class to create for collision + :return: + """ + all_elements = {} + accessible_elements = {} + + for father in self.inheritance: + for element in getter(father): + elem = Cls(element._functionNotParsed, self, element.original_contract) + elem.set_offset(element._functionNotParsed['src'], self.slither) + elem.analyze_params() + self.slither.add_function(elem) + all_elements[elem.canonical_name] = elem + + accessible_elements = self.available_elements_from_inheritances(all_elements, getter_available) # If there is a constructor in the functions # We remove the previous constructor @@ -308,20 +335,25 @@ class ContractSolc04(Contract): # # Note: contract.all_functions_called returns the constructors of the base contracts has_constructor = False - for function in self._functions_no_params: - function.analyze_params() - if function.is_constructor: + for element in elements_no_params: + element.analyze_params() + if element.is_constructor: has_constructor = True if has_constructor: - _functions = {k:v for (k, v) in self._functions.items() if not v.is_constructor} - self._functions = _functions + _accessible_functions = {k: v for (k, v) in accessible_elements.items() if not v.is_constructor} + + for element in elements_no_params: + accessible_elements[element.full_name] = element + all_elements[element.canonical_name] = element + + for element in all_elements.values(): + if accessible_elements[element.full_name] != all_elements[element.canonical_name]: + element.is_shadowed = True + + return all_elements - for function in self._functions_no_params: - self._functions[function.full_name] = function - self._functions_no_params = [] - return def analyze_constant_state_variables(self): from slither.solc_parsing.expressions.expression_parsing import VariableNotFound @@ -434,8 +466,7 @@ class ContractSolc04(Contract): def convert_expression_to_slithir(self): for func in self.functions + self.modifiers: - if func.contract == self: - func.generate_slithir_and_analyze() + func.generate_slithir_and_analyze() all_ssa_state_variables_instances = dict() @@ -453,8 +484,7 @@ class ContractSolc04(Contract): self._initial_state_variables.append(new_var) for func in self.functions + self.modifiers: - if func.contract == self: - func.generate_slithir_ssa(all_ssa_state_variables_instances) + func.generate_slithir_ssa(all_ssa_state_variables_instances) def fix_phi(self): last_state_variables_instances = dict() diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 6b95ca9f9..75580ecac 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -34,9 +34,10 @@ class FunctionSolc(Function): """ # elems = [(type, name)] - def __init__(self, function, contract): + def __init__(self, function, contract, original_contract): super(FunctionSolc, self).__init__() self._contract = contract + self._original_contract = original_contract # Only present if compact AST self._referenced_declaration = None diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 39b2334d6..fd1e6139d 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -68,8 +68,7 @@ def get_pointer_name(variable): return None -def find_variable(var_name, caller_context, referenced_declaration=None): - +def find_variable(var_name, caller_context, referenced_declaration=None, is_super=False): if isinstance(caller_context, Contract): function = None @@ -108,12 +107,21 @@ def find_variable(var_name, caller_context, referenced_declaration=None): if var_name and var_name in conc_variables_ptr: return conc_variables_ptr[var_name] - - functions = contract.functions_as_dict() + if is_super: + getter_available = lambda f: f.available_functions_as_dict().items() + d = {f.canonical_name:f for f in contract.functions} + functions = {f.full_name:f for f in contract.available_elements_from_inheritances(d, getter_available).values()} + else: + functions = contract.available_functions_as_dict() if var_name in functions: return functions[var_name] - modifiers = contract.modifiers_as_dict() + if is_super: + getter_available = lambda m: m.available_modifiers_as_dict().items() + d = {m.canonical_name: m for m in contract.modifiers} + modifiers = {m.full_name: m for m in contract.available_elements_from_inheritances(d, getter_available).values()} + else: + modifiers = contract.available_modifiers_as_dict() if var_name in modifiers: return modifiers[var_name] @@ -516,6 +524,7 @@ def parse_expression(expression, caller_context): referenced_declaration = expression['referencedDeclaration'] else: referenced_declaration = None + var = find_variable(value, caller_context, referenced_declaration) identifier = Identifier(var) @@ -556,18 +565,7 @@ def parse_expression(expression, caller_context): member_expression = parse_expression(children[0], caller_context) if str(member_expression) == 'super': super_name = parse_super_name(expression, is_compact_ast) - if isinstance(caller_context, Contract): - inheritance = caller_context.inheritance - else: - assert isinstance(caller_context, Function) - inheritance = caller_context.contract.inheritance - var = None - for father in inheritance: - try: - var = find_variable(super_name, father) - break - except VariableNotFound: - continue + var = find_variable(super_name, caller_context, is_super=True) if var is None: raise VariableNotFound('Variable not found: {}'.format(super_name)) return SuperIdentifier(var) diff --git a/slither/solc_parsing/solidity_types/type_parsing.py b/slither/solc_parsing/solidity_types/type_parsing.py index 2b5057dda..3dd57c276 100644 --- a/slither/solc_parsing/solidity_types/type_parsing.py +++ b/slither/solc_parsing/solidity_types/type_parsing.py @@ -58,7 +58,7 @@ def _find_from_type_name(name, contract, contracts, structures, enums): all_enums = [item for sublist in all_enums for item in sublist] var_type = next((e for e in all_enums if e.name == enum_name), None) if not var_type: - var_type = next((e for e in all_enums if e.contract.name+"."+e.name == enum_name), None) + var_type = next((e for e in all_enums if e.canonical_name == enum_name), None) if not var_type: # any contract can refer to another contract's structure name_struct = name @@ -69,14 +69,14 @@ def _find_from_type_name(name, contract, contracts, structures, enums): all_structures = [item for sublist in all_structures for item in sublist] var_type = next((st for st in all_structures if st.name == name_struct), None) if not var_type: - var_type = next((st for st in all_structures if st.contract.name+"."+st.name == name_struct), None) + var_type = next((st for st in all_structures if st.canonical_name == name_struct), None) # case where struct xxx.xx[] where not well formed in the AST if not var_type: depth = 0 while name_struct.endswith('[]'): name_struct = name_struct[0:-2] depth+=1 - var_type = next((st for st in all_structures if st.contract.name+"."+st.name == name_struct), None) + var_type = next((st for st in all_structures if st.canonical_name == name_struct), None) if var_type: return ArrayType(UserDefinedType(var_type), Literal(depth)) diff --git a/slither/utils/inheritance_analysis.py b/slither/utils/inheritance_analysis.py index de05b246f..64e62dbc2 100644 --- a/slither/utils/inheritance_analysis.py +++ b/slither/utils/inheritance_analysis.py @@ -109,7 +109,7 @@ def detect_function_shadowing(contracts, direct_shadowing=True, indirect_shadowi for y in range(x + 1, len(colliding_functions)): # The same function definition can appear more than once in the inheritance chain, # overshadowing items between, so it is important to remember to filter it out here. - if colliding_functions[y][1].contract != colliding_functions[x][1].contract: + if colliding_functions[y][1].original_contract != colliding_functions[x][1].original_contract: results.add((contract, colliding_functions[y][0], colliding_functions[y][1], colliding_functions[x][0], colliding_functions[x][1])) diff --git a/utils/possible_paths/__main__.py b/utils/possible_paths/__main__.py index 321f1bcfd..44254a121 100644 --- a/utils/possible_paths/__main__.py +++ b/utils/possible_paths/__main__.py @@ -47,7 +47,7 @@ def main(): # Print out all target functions. print(f"Target functions:") for target in targets: - print(f"- {target.contract.name}.{target.full_name}") + print(f"- {target.original_contract.name}.{target.full_name}") print("\n") # Obtain all paths which reach the target functions. @@ -56,12 +56,12 @@ def main(): # Print out all function names which can reach the targets. print(f"The following functions reach the specified targets:") - for function_desc in sorted([f"{f.contract.name}.{f.full_name}" for f in reaching_functions]): + for function_desc in sorted([f"{f.canonical_name}" for f in reaching_functions]): print(f"- {function_desc}") print("\n") # Format all function paths. - reaching_paths_str = [' -> '.join([f"{f.contract.name}.{f.full_name}" for f in reaching_path]) for reaching_path in reaching_paths] + reaching_paths_str = [' -> '.join([f"{f.canonical_name}" for f in reaching_path]) for reaching_path in reaching_paths] # Print a sorted list of all function paths which can reach the targets. print(f"The following paths reach the specified targets:") diff --git a/utils/upgradeability/check_initialization.py b/utils/upgradeability/check_initialization.py index ac9d29d47..475f3683a 100644 --- a/utils/upgradeability/check_initialization.py +++ b/utils/upgradeability/check_initialization.py @@ -51,7 +51,7 @@ def check_initialization(s): for f in all_init_functions: if not initializer in f.modifiers: initializer_modifier_missing = True - logger.info(red(f'{f.contract.name}.{f.name} does not call initializer')) + logger.info(red(f'{f.canonical_name} does not call initializer')) most_derived_init = _get_most_derived_init(contract) if most_derived_init is None: init_info += f'{contract.name} has no initialize function\n' @@ -61,7 +61,7 @@ def check_initialization(s): all_init_functions_called = _get_all_internal_calls(most_derived_init) + [most_derived_init] missing_calls = [f for f in all_init_functions if not f in all_init_functions_called] for f in missing_calls: - logger.info(red(f'Missing call to {f.contract.name}.{f.name} in {contract.name}')) + logger.info(red(f'Missing call to {f.canonical_name} in {contract.name}')) missing_call = True double_calls = list(set([f for f in all_init_functions_called if all_init_functions_called.count(f) > 1])) for f in double_calls: From 654c2c98b7e69dcfd42ae0c143b886feee8f6951 Mon Sep 17 00:00:00 2001 From: Josselin Date: Fri, 26 Apr 2019 18:33:55 +0100 Subject: [PATCH 03/11] - add contract.get_function_from_canonical_name (same for modifier - slithir convert: fix incorrect internal call conversion (use canonical name) - slithir internalcall: use tuple(func_nale, contract_name) to init unknown internal call - Update upgradability-check with new API --- scripts/travis_test_upgradability.sh | 17 ++++++--- slither/core/declarations/contract.py | 25 ++++++++++++- slither/printers/summary/slithir.py | 2 +- slither/slithir/convert.py | 37 +++++++++++++++---- slither/slithir/operations/internal_call.py | 17 ++++----- slither/slithir/utils/ssa.py | 2 +- .../expressions/expression_parsing.py | 1 + .../visitors/slithir/expression_to_slithir.py | 2 +- tests/check-upgradeability/test_5.txt | 4 +- utils/upgradeability/check_initialization.py | 20 +++++----- 10 files changed, 88 insertions(+), 39 deletions(-) diff --git a/scripts/travis_test_upgradability.sh b/scripts/travis_test_upgradability.sh index adce735c2..d840a2fb3 100755 --- a/scripts/travis_test_upgradability.sh +++ b/scripts/travis_test_upgradability.sh @@ -8,8 +8,9 @@ slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1 DIFF=$(diff test_1.txt "$DIR_TESTS/test_1.txt") if [ "$DIFF" != "" ] then - echo "slither-check-upgradeability failed" + echo "slither-check-upgradeability 1 failed" cat test_1.txt + echo "" cat "$DIR_TESTS/test_1.txt" exit -1 fi @@ -18,8 +19,9 @@ slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1 DIFF=$(diff test_2.txt "$DIR_TESTS/test_2.txt") if [ "$DIFF" != "" ] then - echo "slither-check-upgradeability failed" + echo "slither-check-upgradeability 2 failed" cat test_2.txt + echo "" cat "$DIR_TESTS/test_2.txt" exit -1 fi @@ -28,8 +30,9 @@ slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1 DIFF=$(diff test_3.txt "$DIR_TESTS/test_3.txt") if [ "$DIFF" != "" ] then - echo "slither-check-upgradeability failed" + echo "slither-check-upgradeability 3 failed" cat test_3.txt + echo "" cat "$DIR_TESTS/test_3.txt" exit -1 fi @@ -38,8 +41,9 @@ slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1 DIFF=$(diff test_4.txt "$DIR_TESTS/test_4.txt") if [ "$DIFF" != "" ] then - echo "slither-check-upgradeability failed" + echo "slither-check-upgradeability 4 failed" cat test_4.txt + echo "" cat "$DIR_TESTS/test_4.txt" exit -1 fi @@ -48,9 +52,12 @@ slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contract_i DIFF=$(diff test_5.txt "$DIR_TESTS/test_5.txt") if [ "$DIFF" != "" ] then - echo "slither-check-upgradeability failed" + echo "slither-check-upgradeability 5 failed" cat test_5.txt + echo "" cat "$DIR_TESTS/test_5.txt" + echo "" + echo "$DIFF" exit -1 fi diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 5f1c0a2e9..9ef2e88bf 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -402,7 +402,7 @@ class Contract(ChildSlither, SourceMapping): Returns: Function """ - return next((f for f in self.functions if f.full_name == function_signature), None) + return next((f for f in self.functions if f.full_name == function_signature and not f.is_shadowed), None) def get_modifier_from_signature(self, modifier_signature): """ @@ -412,7 +412,28 @@ class Contract(ChildSlither, SourceMapping): Returns: Modifier """ - return next((m for m in self.modifiers if m.full_name == modifier_signature), None) + return next((m for m in self.modifiers if m.full_name == modifier_signature and not m.is_shadowed), None) + + def get_function_from_canonical_name(self, canonical_name): + """ + Return a function from a a canonical name (contract.signature()) + Args: + canonical_name (str): canonical name of the function (without return statement) + Returns: + Function + """ + return next((f for f in self.functions if f.canonical_name == canonical_name), None) + + def get_modifier_from_canonical_name(self, canonical_name): + """ + Return a modifier from a canonical name (contract.signature()) + Args: + canonical_name (str): canonical name of the modifier + Returns: + Modifier + """ + return next((m for m in self.modifiers if m.canonical_name == canonical_name), None) + def get_state_variable_from_name(self, variable_name): """ diff --git a/slither/printers/summary/slithir.py b/slither/printers/summary/slithir.py index e34c9d34d..cd4a7299f 100644 --- a/slither/printers/summary/slithir.py +++ b/slither/printers/summary/slithir.py @@ -23,7 +23,7 @@ class PrinterSlithIR(AbstractPrinter): for contract in self.contracts: print('Contract {}'.format(contract.name)) for function in contract.functions: - print('\tFunction {}'.format(function.canonical_name)) + print(f'\tFunction {function.canonical_name}') for node in function.nodes: if node.expression: print('\t\tExpression: {}'.format(node.expression)) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 7b9607df0..51993b2c6 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -98,6 +98,21 @@ def get_sig(ir, name): argss = convert_arguments(ir.arguments) return [sig.format(name, ','.join(args)) for args in argss] +def get_canonical_names(ir, function_name, contract_name): + ''' + Return a list of potential signature + It is a list, as Constant variables can be converted to int256 + Args: + ir (slithIR.operation) + Returns: + list(str) + ''' + sig = '{}({})' + + # list of list of arguments + argss = convert_arguments(ir.arguments) + return [sig.format(f'{contract_name}.{function_name}', ','.join(args)) for args in argss] + def convert_arguments(arguments): argss = [[]] for arg in arguments: @@ -355,7 +370,7 @@ def propagate_types(ir, node): elif isinstance(ir, InternalCall): # if its not a tuple, return a singleton if ir.function is None: - convert_type_of_high_and_internal_level_call(ir, ir.contract) + convert_type_of_high_and_internal_level_call(ir, node.function.contract) return_type = ir.function.return_type if return_type: if len(return_type) == 1: @@ -467,7 +482,7 @@ def extract_tmp_call(ins, contract): # If there is a call on an inherited contract, it is an internal call or an event if ins.ori.variable_left in contract.inheritance + [contract]: if str(ins.ori.variable_right) in [f.name for f in contract.functions]: - internalcall = InternalCall(ins.ori.variable_right, ins.ori.variable_left, ins.nbr_arguments, ins.lvalue, ins.type_call) + internalcall = InternalCall((ins.ori.variable_right, ins.ori.variable_left.name), ins.nbr_arguments, ins.lvalue, ins.type_call) internalcall.call_id = ins.call_id return internalcall if str(ins.ori.variable_right) in [f.name for f in contract.events]: @@ -704,11 +719,19 @@ def convert_type_library_call(ir, lib_contract): def convert_type_of_high_and_internal_level_call(ir, contract): func = None - sigs = get_sig(ir, ir.function_name) - for sig in sigs: - func = contract.get_function_from_signature(sig) - if not func: - func = contract.get_state_variable_from_name(ir.function_name) + if isinstance(ir, InternalCall): + sigs = get_canonical_names(ir, ir.function_name, ir.contract_name) + for sig in sigs: + func = contract.get_function_from_canonical_name(sig) + if not func: + func = contract.get_state_variable_from_name(ir.function_name) + else: + assert isinstance(ir, HighLevelCall) + sigs = get_sig(ir, ir.function_name) + for sig in sigs: + func = contract.get_function_from_canonical_name(sig) + if not func: + func = contract.get_state_variable_from_name(ir.function_name) if not func: # specific lookup when the compiler does implicit conversion # for example diff --git a/slither/slithir/operations/internal_call.py b/slither/slithir/operations/internal_call.py index c0e5b54f5..75c5d6a9d 100644 --- a/slither/slithir/operations/internal_call.py +++ b/slither/slithir/operations/internal_call.py @@ -1,21 +1,20 @@ from slither.core.declarations.function import Function from slither.slithir.operations.call import Call from slither.slithir.operations.lvalue import OperationWithLValue -from slither.core.variables.variable import Variable from slither.slithir.variables import Constant class InternalCall(Call, OperationWithLValue): - def __init__(self, function, contract, nbr_arguments, result, type_call): + def __init__(self, function, nbr_arguments, result, type_call): super(InternalCall, self).__init__() if isinstance(function, Function): self._function = function self._function_name = function.name + self._contract_name = function.original_contract.name else: - isinstance(function, Constant) self._function = None - self._function_name = function - self._contract = contract + self._function_name, self._contract_name = function + #self._contract = contract self._nbr_arguments = nbr_arguments self._type_call = type_call self._lvalue = result @@ -32,14 +31,14 @@ class InternalCall(Call, OperationWithLValue): def function(self, f): self._function = f - @property - def contract(self): - return self._contract - @property def function_name(self): return self._function_name + @property + def contract_name(self): + return self._contract_name + @property def nbr_arguments(self): return self._nbr_arguments diff --git a/slither/slithir/utils/ssa.py b/slither/slithir/utils/ssa.py index 74238589d..3ef917dbd 100644 --- a/slither/slithir/utils/ssa.py +++ b/slither/slithir/utils/ssa.py @@ -565,7 +565,7 @@ def copy_ir(ir, *instances): nbr_arguments = ir.nbr_arguments lvalue = get_variable(ir, lambda x: x.lvalue, *instances) type_call = ir.type_call - new_ir = InternalCall(function, function.contract, nbr_arguments, lvalue, type_call) + new_ir = InternalCall(function, nbr_arguments, lvalue, type_call) new_ir.arguments = get_arguments(ir, *instances) return new_ir elif isinstance(ir, InternalDynamicCall): diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index fd1e6139d..760d6b9a5 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -70,6 +70,7 @@ def get_pointer_name(variable): def find_variable(var_name, caller_context, referenced_declaration=None, is_super=False): + if isinstance(caller_context, Contract): function = None contract = caller_context diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index a1c1c5e5d..044f862bf 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -131,7 +131,7 @@ class ExpressionToSlithIR(ExpressionVisitor): val = TupleVariable(self._node) else: val = TemporaryVariable(self._node) - internal_call = InternalCall(called, called.contract, len(args), val, expression.type_call) + internal_call = InternalCall(called, len(args), val, expression.type_call) self._result.append(internal_call) set_val(expression, val) else: diff --git a/tests/check-upgradeability/test_5.txt b/tests/check-upgradeability/test_5.txt index 8ddb49d85..fc7bf6e2c 100644 --- a/tests/check-upgradeability/test_5.txt +++ b/tests/check-upgradeability/test_5.txt @@ -1,6 +1,6 @@ INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks) -INFO:CheckInitialization:Contract_lack_to_call_modifier.initialize does not call initializer -INFO:CheckInitialization:Missing call to Contract_no_bug.initialize in Contract_not_called_super_init +INFO:CheckInitialization:Contract_lack_to_call_modifier.initialize() does not call initializer +INFO:CheckInitialization:Missing call to Contract_no_bug.initialize() in Contract_not_called_super_init INFO:CheckInitialization:Contract_no_bug.initialize() is called multiple time in Contract_double_call INFO:CheckInitialization:Check the deployement script to ensure that these functions are called: Contract_no_bug needs to be initialized by initialize() diff --git a/utils/upgradeability/check_initialization.py b/utils/upgradeability/check_initialization.py index 475f3683a..22885ed31 100644 --- a/utils/upgradeability/check_initialization.py +++ b/utils/upgradeability/check_initialization.py @@ -11,7 +11,7 @@ class MultipleInitTarget(Exception): pass def _get_initialize_functions(contract): - return [f for father in contract.inheritance + [contract] for f in father.functions_not_inherited if f.name == 'initialize'] + return [f for f in contract.functions if f.name == 'initialize'] def _get_all_internal_calls(function): all_ir = function.all_slithir_operations() @@ -19,12 +19,11 @@ def _get_all_internal_calls(function): def _get_most_derived_init(contract): - for c in [contract] + contract.inheritance: - init_functions = [f for f in c.functions_not_inherited if f.name == 'initialize'] - if len(init_functions) > 1: - raise MultipleInitTarget - if init_functions: - return init_functions[0] + init_functions = [f for f in contract.functions if not f.is_shadowed and f.name == 'initialize'] + if len(init_functions) > 1: + raise MultipleInitTarget + if init_functions: + return init_functions[0] return None def check_initialization(s): @@ -37,8 +36,6 @@ def check_initialization(s): logger.info(yellow('Initializable contract not found, the contract does not follow a standard initalization schema.')) return - initializer = initializable.get_modifier_from_signature('initializer()') - init_info = '' double_calls_found = False @@ -47,6 +44,7 @@ def check_initialization(s): for contract in s.contracts: if initializable in contract.inheritance: + initializer = contract.get_modifier_from_canonical_name('Initializable.initializer()') all_init_functions = _get_initialize_functions(contract) for f in all_init_functions: if not initializer in f.modifiers: @@ -58,14 +56,14 @@ def check_initialization(s): continue else: init_info += f'{contract.name} needs to be initialized by {most_derived_init.full_name}\n' - all_init_functions_called = _get_all_internal_calls(most_derived_init) + [most_derived_init] + all_init_functions_called = _get_all_internal_calls(most_derived_init) + [most_derived_init] missing_calls = [f for f in all_init_functions if not f in all_init_functions_called] for f in missing_calls: logger.info(red(f'Missing call to {f.canonical_name} in {contract.name}')) missing_call = True double_calls = list(set([f for f in all_init_functions_called if all_init_functions_called.count(f) > 1])) for f in double_calls: - logger.info(red(f'{f.contract.name + "." + f.full_name} is called multiple time in {contract.name}')) + logger.info(red(f'{f.canonical_name} is called multiple time in {contract.name}')) double_calls_found = True if not initializer_modifier_missing: From 92ced37b30b8e7182cd35851fad7f45b08d9c377 Mon Sep 17 00:00:00 2001 From: Josselin Date: Fri, 26 Apr 2019 19:30:18 +0100 Subject: [PATCH 04/11] Fix contract.all_functions_called --- slither/core/declarations/contract.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 9ef2e88bf..8a022ef86 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -520,8 +520,9 @@ class Contract(ChildSlither, SourceMapping): ''' list(Function): List of functions reachable from the contract (include super) ''' - all_calls = [f.all_internal_calls() for f in self.functions + self.modifiers] + [self.functions + self.modifiers] - all_calls = [item for sublist in all_calls for item in sublist] + self.functions + all_calls = [f for f in self.functions + self.modifiers if not f.is_shadowed] + all_calls = [f.all_internal_calls() for f in all_calls] + [all_calls] + all_calls = [item for sublist in all_calls for item in sublist] all_calls = list(set(all_calls)) all_constructors = [c.constructor for c in self.inheritance] From d37bcb6104630a4202e631a2f06659e0a076d89a Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 29 Apr 2019 11:10:00 +0100 Subject: [PATCH 05/11] Rename original_contract -> contract_declarer --- examples/scripts/slithIR.py | 2 +- slither/core/children/child_contract.py | 1 - slither/core/children/child_inheritance.py | 10 +++++----- slither/core/declarations/contract.py | 13 +++++++------ slither/core/declarations/function.py | 6 +++--- slither/detectors/abstract_detector.py | 2 +- slither/detectors/attributes/const_functions.py | 2 +- slither/detectors/erc20/incorrect_interface.py | 2 +- slither/detectors/functions/arbitrary_send.py | 2 +- slither/detectors/functions/suicidal.py | 2 +- .../naming_convention/naming_convention.py | 4 ++-- slither/detectors/operations/block_timestamp.py | 2 +- slither/detectors/operations/low_level_calls.py | 2 +- .../detectors/operations/unused_return_values.py | 2 +- slither/detectors/shadowing/builtin_symbols.py | 4 ++-- slither/detectors/shadowing/local.py | 6 +++--- slither/detectors/statements/assembly.py | 2 +- slither/detectors/statements/calls_in_loop.py | 2 +- .../detectors/statements/controlled_delegatecall.py | 2 +- .../variables/uninitialized_local_variables.py | 2 +- slither/printers/inheritance/inheritance_graph.py | 6 +++--- slither/slithir/operations/internal_call.py | 2 +- slither/solc_parsing/declarations/contract.py | 4 ++-- slither/solc_parsing/declarations/function.py | 4 ++-- slither/utils/inheritance_analysis.py | 2 +- utils/possible_paths/__main__.py | 2 +- 26 files changed, 45 insertions(+), 45 deletions(-) diff --git a/examples/scripts/slithIR.py b/examples/scripts/slithIR.py index b58e06f10..2b65e3122 100644 --- a/examples/scripts/slithIR.py +++ b/examples/scripts/slithIR.py @@ -15,7 +15,7 @@ for contract in slither.contracts: for function in contract.functions: # Dont explore inherited functions - if function.original_contract == contract: + if function.contract_declarer == contract: print('Function: {}'.format(function.name)) diff --git a/slither/core/children/child_contract.py b/slither/core/children/child_contract.py index 6e476d59f..9ca39af8e 100644 --- a/slither/core/children/child_contract.py +++ b/slither/core/children/child_contract.py @@ -4,7 +4,6 @@ class ChildContract: def __init__(self): super(ChildContract, self).__init__() self._contract = None - self._original_contract = None def set_contract(self, contract): self._contract = contract diff --git a/slither/core/children/child_inheritance.py b/slither/core/children/child_inheritance.py index 668c37a5f..cc9c4065f 100644 --- a/slither/core/children/child_inheritance.py +++ b/slither/core/children/child_inheritance.py @@ -3,11 +3,11 @@ class ChildInheritance: def __init__(self): super(ChildInheritance, self).__init__() - self._original_contract = None + self._contract_declarer = None - def set_original_contract(self, original_contract): - self._original_contract = original_contract + def set_contract_declarer(self, contract): + self._contract_declarer = contract @property - def original_contract(self): - return self._original_contract + def contract_declarer(self): + return self._contract_declarer diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 8a022ef86..a116f22c1 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -184,7 +184,7 @@ class Contract(ChildSlither, SourceMapping): @property def constructor_not_inherited(self): - return next((func for func in self.functions if func.is_constructor and func.original_contract == self), None) + return next((func for func in self.functions if func.is_constructor and func.contract_declarer == self), None) @property def constructors(self): @@ -228,14 +228,14 @@ class Contract(ChildSlither, SourceMapping): ''' list(Function): List of the inherited functions ''' - return [f for f in self.functions if f.original_contract != self] + return [f for f in self.functions if f.contract_declarer != self] @property def functions_not_inherited(self): ''' list(Function): List of the functions defined within the contract (not inherited) ''' - return [f for f in self.functions if f.original_contract == self] + return [f for f in self.functions if f.contract_declarer == self] @property def functions_entry_points(self): @@ -259,14 +259,14 @@ class Contract(ChildSlither, SourceMapping): ''' list(Modifier): List of the inherited modifiers ''' - return [m for m in self.modifiers if m.original_contract != self] + return [m for m in self.modifiers if m.contract_declarer != self] @property def modifiers_not_inherited(self): ''' list(Modifier): List of the modifiers defined within the contract (not inherited) ''' - return [m for m in self.modifiers if m.original_contract == self] + return [m for m in self.modifiers if m.contract_declarer == self] @property def functions_and_modifiers(self): @@ -518,7 +518,8 @@ class Contract(ChildSlither, SourceMapping): @property def all_functions_called(self): ''' - list(Function): List of functions reachable from the contract (include super) + list(Function): List of functions reachable from the contract + Includes super, and private/internal functions not shadowed ''' all_calls = [f for f in self.functions + self.modifiers if not f.is_shadowed] all_calls = [f.all_internal_calls() for f in all_calls] + [all_calls] diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index 27ffa6996..fa0542ea3 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -123,14 +123,14 @@ class Function(ChildContract, ChildInheritance, SourceMapping): Return the function signature without the return values """ name, parameters, _ = self.signature - return self.original_contract.name + '.' + name + '(' + ','.join(parameters) + ')' + return self.contract_declarer.name + '.' + name + '(' + ','.join(parameters) + ')' @property def is_constructor(self): """ bool: True if the function is the constructor """ - return self._is_constructor or self._name == self.original_contract.name + return self._is_constructor or self._name == self.contract_declarer.name @property def contains_assembly(self): @@ -950,7 +950,7 @@ class Function(ChildContract, ChildInheritance, SourceMapping): (str, str, str, list(str), list(str), listr(str), list(str), list(str); contract_name, name, visibility, modifiers, vars read, vars written, internal_calls, external_calls_as_expressions """ - return (self.original_contract.name, self.full_name, self.visibility, + return (self.contract_declarer.name, self.full_name, self.visibility, [str(x) for x in self.modifiers], [str(x) for x in self.state_variables_read + self.solidity_variables_read], [str(x) for x in self.state_variables_written], diff --git a/slither/detectors/abstract_detector.py b/slither/detectors/abstract_detector.py index 37324f80a..9b7fa20c0 100644 --- a/slither/detectors/abstract_detector.py +++ b/slither/detectors/abstract_detector.py @@ -169,7 +169,7 @@ class AbstractDetector(metaclass=abc.ABCMeta): @staticmethod def add_function_to_json(function, d): contract = {'elements':[]} - AbstractDetector.add_contract_to_json(function.original_contract, contract) + AbstractDetector.add_contract_to_json(function.contract_declarer, contract) d['elements'].append({'type': 'function', 'name': function.name, 'source_mapping': function.source_mapping, diff --git a/slither/detectors/attributes/const_functions.py b/slither/detectors/attributes/const_functions.py index f38693f77..1b3b95319 100644 --- a/slither/detectors/attributes/const_functions.py +++ b/slither/detectors/attributes/const_functions.py @@ -51,7 +51,7 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' results = [] for c in self.contracts: for f in c.functions: - if f.original_contract != c: + if f.contract_declarer != c: continue if f.view or f.pure: if f.contains_assembly: diff --git a/slither/detectors/erc20/incorrect_interface.py b/slither/detectors/erc20/incorrect_interface.py index dbd0feb92..0c7bb6bd5 100644 --- a/slither/detectors/erc20/incorrect_interface.py +++ b/slither/detectors/erc20/incorrect_interface.py @@ -52,7 +52,7 @@ contract Token{ Returns: list(str) : list of incorrect function signatures """ - functions = [f for f in contract.functions if f.original_contract == contract and \ + functions = [f for f in contract.functions if f.contract_declarer == contract and \ IncorrectERC20InterfaceDetection.incorrect_erc20_interface(f.signature)] return functions diff --git a/slither/detectors/functions/arbitrary_send.py b/slither/detectors/functions/arbitrary_send.py index e5900a3c5..d28cd1258 100644 --- a/slither/detectors/functions/arbitrary_send.py +++ b/slither/detectors/functions/arbitrary_send.py @@ -94,7 +94,7 @@ Bob calls `setDestination` and `withdraw`. As a result he withdraws the contract list((Function), (list (Node))) """ ret = [] - for f in [f for f in contract.functions if f.original_contract == contract]: + for f in [f for f in contract.functions if f.contract_declarer == contract]: nodes = self.arbitrary_send(f) if nodes: ret.append((f, nodes)) diff --git a/slither/detectors/functions/suicidal.py b/slither/detectors/functions/suicidal.py index f20c4da13..fef1c1224 100644 --- a/slither/detectors/functions/suicidal.py +++ b/slither/detectors/functions/suicidal.py @@ -59,7 +59,7 @@ Bob calls `kill` and destructs the contract.''' def detect_suicidal(self, contract): ret = [] - for f in [f for f in contract.functions if f.original_contract == contract]: + for f in [f for f in contract.functions if f.contract_declarer == contract]: if self.detect_suicidal_func(f): ret.append(f) return ret diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index 4ec7fc6d1..b74696f34 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -103,7 +103,7 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 results.append(json) for func in contract.functions: - if func.original_contract != contract: + if func.contract_declarer != contract: continue if not self.is_mixed_case(func.name): @@ -212,7 +212,7 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 for modifier in contract.modifiers: - if modifier.original_contract != contract: + if modifier.contract_declarer != contract: continue if not self.is_mixed_case(modifier.name): diff --git a/slither/detectors/operations/block_timestamp.py b/slither/detectors/operations/block_timestamp.py index ecadb2e94..81c115341 100644 --- a/slither/detectors/operations/block_timestamp.py +++ b/slither/detectors/operations/block_timestamp.py @@ -54,7 +54,7 @@ class Timestamp(AbstractDetector): list((Function), (list (Node))) """ ret = [] - for f in [f for f in contract.functions if f.original_contract == contract]: + for f in [f for f in contract.functions if f.contract_declarer == contract]: nodes = self.timestamp(f) if nodes: ret.append((f, nodes)) diff --git a/slither/detectors/operations/low_level_calls.py b/slither/detectors/operations/low_level_calls.py index 9a819518d..4e36448a4 100644 --- a/slither/detectors/operations/low_level_calls.py +++ b/slither/detectors/operations/low_level_calls.py @@ -33,7 +33,7 @@ class LowLevelCalls(AbstractDetector): def detect_low_level_calls(self, contract): ret = [] - for f in [f for f in contract.functions if contract == f.original_contract]: + for f in [f for f in contract.functions if contract == f.contract_declarer]: nodes = f.nodes assembly_nodes = [n for n in nodes if self._contains_low_level_calls(n)] diff --git a/slither/detectors/operations/unused_return_values.py b/slither/detectors/operations/unused_return_values.py index ee153f03a..1bf9ef3d2 100644 --- a/slither/detectors/operations/unused_return_values.py +++ b/slither/detectors/operations/unused_return_values.py @@ -64,7 +64,7 @@ contract MyConc{ results = [] for c in self.slither.contracts: for f in c.functions + c.modifiers: - if f.original_contract != c: + if f.contract_declarer != c: continue unused_return = self.detect_unused_return_values(f) if unused_return: diff --git a/slither/detectors/shadowing/builtin_symbols.py b/slither/detectors/shadowing/builtin_symbols.py index cc8a1f19e..d04f24d54 100644 --- a/slither/detectors/shadowing/builtin_symbols.py +++ b/slither/detectors/shadowing/builtin_symbols.py @@ -91,12 +91,12 @@ contract Bug { # Loop through all functions, modifiers, variables (state and local) to detect any built-in symbol keywords. for function in contract.functions: - if function.original_contract == contract: + if function.contract_declarer == contract: if self.is_builtin_symbol(function.name): result.append((self.SHADOWING_FUNCTION, function, None)) result += self.detect_builtin_shadowing_locals(function) for modifier in contract.modifiers: - if modifier.original_contract == contract: + if modifier.contract_declarer == contract: if self.is_builtin_symbol(modifier.name): result.append((self.SHADOWING_MODIFIER, modifier, None)) result += self.detect_builtin_shadowing_locals(modifier) diff --git a/slither/detectors/shadowing/local.py b/slither/detectors/shadowing/local.py index 20d5cebc7..aadc637b9 100644 --- a/slither/detectors/shadowing/local.py +++ b/slither/detectors/shadowing/local.py @@ -59,7 +59,7 @@ contract Bug { # Loop through all functions + modifiers in this contract. for function in contract.functions + contract.modifiers: # We should only look for functions declared directly in this contract (not in a base contract). - if function.original_contract != contract: + if function.contract_declarer != contract: continue # This function was declared in this contract, we check what its local variables might shadow. @@ -68,11 +68,11 @@ contract Bug { for scope_contract in [contract] + contract.inheritance: # Check functions for scope_function in scope_contract.functions: - if variable.name == scope_function.name and scope_function.original_contract == scope_contract: + if variable.name == scope_function.name and scope_function.contract_declarer == scope_contract: overshadowed.append((self.OVERSHADOWED_FUNCTION, scope_contract.name, scope_function)) # Check modifiers for scope_modifier in scope_contract.modifiers: - if variable.name == scope_modifier.name and scope_modifier.original_contract == scope_contract: + if variable.name == scope_modifier.name and scope_modifier.contract_declarer == scope_contract: overshadowed.append((self.OVERSHADOWED_MODIFIER, scope_contract.name, scope_modifier)) # Check events for scope_event in scope_contract.events: diff --git a/slither/detectors/statements/assembly.py b/slither/detectors/statements/assembly.py index d95a81731..936794366 100644 --- a/slither/detectors/statements/assembly.py +++ b/slither/detectors/statements/assembly.py @@ -35,7 +35,7 @@ class Assembly(AbstractDetector): def detect_assembly(self, contract): ret = [] for f in contract.functions: - if f.original_contract != contract: + if f.contract_declarer != contract: continue nodes = f.nodes assembly_nodes = [n for n in nodes if diff --git a/slither/detectors/statements/calls_in_loop.py b/slither/detectors/statements/calls_in_loop.py index b3fa39f26..223ddbb42 100644 --- a/slither/detectors/statements/calls_in_loop.py +++ b/slither/detectors/statements/calls_in_loop.py @@ -72,7 +72,7 @@ If one of the destinations has a fallback function which reverts, `bad` will alw def detect_call_in_loop(contract): ret = [] for f in contract.functions + contract.modifiers: - if f.original_contract == contract and f.is_implemented: + if f.contract_declarer == contract and f.is_implemented: MultipleCallsInLoop.call_in_loop(f.entry_point, False, [], ret) diff --git a/slither/detectors/statements/controlled_delegatecall.py b/slither/detectors/statements/controlled_delegatecall.py index 0f721426e..81a25f71d 100644 --- a/slither/detectors/statements/controlled_delegatecall.py +++ b/slither/detectors/statements/controlled_delegatecall.py @@ -42,7 +42,7 @@ Bob calls `delegate` and delegates the execution to its malicious contract. As a for contract in self.slither.contracts: for f in contract.functions: - if f.original_contract != contract: + if f.contract_declarer != contract: continue nodes = self.controlled_delegatecall(f) if nodes: diff --git a/slither/detectors/variables/uninitialized_local_variables.py b/slither/detectors/variables/uninitialized_local_variables.py index 289f844d5..39219ae4a 100644 --- a/slither/detectors/variables/uninitialized_local_variables.py +++ b/slither/detectors/variables/uninitialized_local_variables.py @@ -90,7 +90,7 @@ Bob calls `transfer`. As a result, the ethers are sent to the address 0x0 and ar for contract in self.slither.contracts: for function in contract.functions: - if function.is_implemented and function.original_contract == contract: + if function.is_implemented and function.contract_declarer == contract: if function.contains_assembly: continue # dont consider storage variable, as they are detected by another detector diff --git a/slither/printers/inheritance/inheritance_graph.py b/slither/printers/inheritance/inheritance_graph.py index fb6e7d834..75c60b5e1 100644 --- a/slither/printers/inheritance/inheritance_graph.py +++ b/slither/printers/inheritance/inheritance_graph.py @@ -116,14 +116,14 @@ class PrinterInheritanceGraph(AbstractPrinter): # Functions visibilities = ['public', 'external'] public_functions = [self._get_pattern_func(f, contract) for f in contract.functions if - not f.is_constructor and f.original_contract == contract and f.visibility in visibilities] + not f.is_constructor and f.contract_declarer == contract and f.visibility in visibilities] public_functions = ''.join(public_functions) private_functions = [self._get_pattern_func(f, contract) for f in contract.functions if - not f.is_constructor and f.original_contract == contract and f.visibility not in visibilities] + not f.is_constructor and f.contract_declarer == contract and f.visibility not in visibilities] private_functions = ''.join(private_functions) # Modifiers - modifiers = [self._get_pattern_func(m, contract) for m in contract.modifiers if m.original_contract == contract] + modifiers = [self._get_pattern_func(m, contract) for m in contract.modifiers if m.contract_declarer == contract] modifiers = ''.join(modifiers) # Public variables diff --git a/slither/slithir/operations/internal_call.py b/slither/slithir/operations/internal_call.py index 75c5d6a9d..8056695f3 100644 --- a/slither/slithir/operations/internal_call.py +++ b/slither/slithir/operations/internal_call.py @@ -10,7 +10,7 @@ class InternalCall(Call, OperationWithLValue): if isinstance(function, Function): self._function = function self._function_name = function.name - self._contract_name = function.original_contract.name + self._contract_name = function.contract_declarer.name else: self._function = None self._function_name, self._contract_name = function diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 500b67a9c..27de7b4b1 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -234,7 +234,7 @@ class ContractSolc04(Contract): modif = ModifierSolc(modifier, self, self) modif.set_contract(self) - modif.set_original_contract(self) + modif.set_contract_declarer(self) modif.set_offset(modifier['src'], self.slither) self.slither.add_modifier(modif) self._modifiers_no_params.append(modif) @@ -321,7 +321,7 @@ class ContractSolc04(Contract): for father in self.inheritance: for element in getter(father): - elem = Cls(element._functionNotParsed, self, element.original_contract) + elem = Cls(element._functionNotParsed, self, element.contract_declarer) elem.set_offset(element._functionNotParsed['src'], self.slither) elem.analyze_params() self.slither.add_function(elem) diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py index 75580ecac..290f3d929 100644 --- a/slither/solc_parsing/declarations/function.py +++ b/slither/solc_parsing/declarations/function.py @@ -34,10 +34,10 @@ class FunctionSolc(Function): """ # elems = [(type, name)] - def __init__(self, function, contract, original_contract): + def __init__(self, function, contract, contract_declarer): super(FunctionSolc, self).__init__() self._contract = contract - self._original_contract = original_contract + self._contract_declarer = contract_declarer # Only present if compact AST self._referenced_declaration = None diff --git a/slither/utils/inheritance_analysis.py b/slither/utils/inheritance_analysis.py index 64e62dbc2..0c0f7c6da 100644 --- a/slither/utils/inheritance_analysis.py +++ b/slither/utils/inheritance_analysis.py @@ -109,7 +109,7 @@ def detect_function_shadowing(contracts, direct_shadowing=True, indirect_shadowi for y in range(x + 1, len(colliding_functions)): # The same function definition can appear more than once in the inheritance chain, # overshadowing items between, so it is important to remember to filter it out here. - if colliding_functions[y][1].original_contract != colliding_functions[x][1].original_contract: + if colliding_functions[y][1].contract_declarer != colliding_functions[x][1].contract_declarer: results.add((contract, colliding_functions[y][0], colliding_functions[y][1], colliding_functions[x][0], colliding_functions[x][1])) diff --git a/utils/possible_paths/__main__.py b/utils/possible_paths/__main__.py index 44254a121..8e05a29b7 100644 --- a/utils/possible_paths/__main__.py +++ b/utils/possible_paths/__main__.py @@ -47,7 +47,7 @@ def main(): # Print out all target functions. print(f"Target functions:") for target in targets: - print(f"- {target.original_contract.name}.{target.full_name}") + print(f"- {target.contract_declarer.name}.{target.full_name}") print("\n") # Obtain all paths which reach the target functions. From 5d5466e59ffba4c697b6942bab8c921f42d6b492 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 29 Apr 2019 13:30:59 +0100 Subject: [PATCH 06/11] Fix incorrect variable lookup --- slither/core/variables/state_variable.py | 4 +++- .../expressions/expression_parsing.py | 22 +++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/slither/core/variables/state_variable.py b/slither/core/variables/state_variable.py index adca10c19..86c21e30e 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -6,4 +6,6 @@ class StateVariable(ChildContract, Variable): @property def canonical_name(self): - return '{}:{}'.format(self.contract.name, self.name) + return '{}.{}'.format(self.contract.name, self.name) + + diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py index 760d6b9a5..f7614b3ce 100644 --- a/slither/solc_parsing/expressions/expression_parsing.py +++ b/slither/solc_parsing/expressions/expression_parsing.py @@ -70,13 +70,29 @@ def get_pointer_name(variable): def find_variable(var_name, caller_context, referenced_declaration=None, is_super=False): + # variable are looked from the contract declarer + # functions can be shadowed, but are looked from the contract instance, rather than the contract declarer + # the difference between function and variable come from the fact that an internal call, or an variable access + # in a function does not behave similariy, for example in: + # contract C{ + # function f(){ + # state_var = 1 + # f2() + # } + # state_var will refer to C.state_var, no mater if C is inherited + # while f2() will refer to the function definition of the inherited contract (C.f2() in the context of C, or + # the contract inheriting from C) + # for events it's unclear what should be the behavior, as they can be shadowed, but there is not impact + # structure/enums cannot be shadowed if isinstance(caller_context, Contract): function = None contract = caller_context + contract_declarer = caller_context elif isinstance(caller_context, Function): function = caller_context contract = function.contract + contract_declarer = function.contract_declarer else: logger.error('Incorrect caller context') exit(-1) @@ -99,12 +115,13 @@ def find_variable(var_name, caller_context, referenced_declaration=None, is_supe if var_name and var_name in func_variables_ptr: return func_variables_ptr[var_name] - contract_variables = contract.variables_as_dict() + # variable are looked from the contract declarer + contract_variables = contract_declarer.variables_as_dict() if var_name in contract_variables: return contract_variables[var_name] # A state variable can be a pointer - conc_variables_ptr = {get_pointer_name(f) : f for f in contract.variables} + conc_variables_ptr = {get_pointer_name(f) : f for f in contract_declarer.variables} if var_name and var_name in conc_variables_ptr: return conc_variables_ptr[var_name] @@ -126,6 +143,7 @@ def find_variable(var_name, caller_context, referenced_declaration=None, is_supe if var_name in modifiers: return modifiers[var_name] + # structures are looked on the contract declarer structures = contract.structures_as_dict() if var_name in structures: return structures[var_name] From 8af18a884ec51e983878780af31c585c3e0beec6 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 29 Apr 2019 15:49:51 +0100 Subject: [PATCH 07/11] API change: contracts.*not_inherited -> contracts.*_declared Add *_inherited *_declared for events/enums/state_variables/structures Use *_declared in core and detectors (remove *.contract == contract type of check) --- examples/scripts/possible_paths.py | 4 +- slither/core/declarations/contract.py | 74 ++++++++++++++++--- slither/core/declarations/enum.py | 8 ++ slither/core/declarations/event.py | 8 ++ slither/core/declarations/function.py | 12 ++- slither/core/declarations/structure.py | 8 ++ slither/core/variables/variable.py | 8 ++ .../erc20/unindexed_event_parameters.py | 6 +- .../detectors/functions/external_function.py | 4 +- .../naming_convention/naming_convention.py | 35 ++------- slither/detectors/reentrancy/reentrancy.py | 2 +- .../detectors/reentrancy/reentrancy_benign.py | 2 +- .../detectors/reentrancy/reentrancy_eth.py | 2 +- .../reentrancy_read_before_write.py | 2 +- slither/detectors/shadowing/abstract.py | 12 ++- .../detectors/shadowing/builtin_symbols.py | 32 ++++---- slither/detectors/shadowing/local.py | 16 ++-- slither/detectors/shadowing/state.py | 4 +- .../detectors/statements/deprecated_calls.py | 10 +-- .../printers/inheritance/inheritance_graph.py | 12 +-- slither/printers/summary/data_depenency.py | 2 +- slither/slithir/convert.py | 5 +- slither/solc_parsing/declarations/contract.py | 9 +-- slither/utils/inheritance_analysis.py | 5 +- utils/possible_paths/possible_paths.py | 4 +- 25 files changed, 174 insertions(+), 112 deletions(-) diff --git a/examples/scripts/possible_paths.py b/examples/scripts/possible_paths.py index 6c3b6d0be..87806520f 100644 --- a/examples/scripts/possible_paths.py +++ b/examples/scripts/possible_paths.py @@ -69,7 +69,7 @@ def all_function_definitions(function): :return: Returns a list composed of the provided function definition and any base definitions. """ return [function] + [f for c in function.contract.inheritance - for f in c.functions_and_modifiers_not_inherited + for f in c.functions_and_modifiers_declared if f.full_name == function.full_name] @@ -86,7 +86,7 @@ def __find_target_paths(target_function, current_path=[]): # Look through all functions for contract in slither.contracts: - for function in contract.functions_and_modifiers_not_inherited: + for function in contract.functions_and_modifiers_declared: # If the function is already in our path, skip it. if function in current_path: diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index a116f22c1..a79a70e4e 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -73,6 +73,20 @@ class Contract(ChildSlither, SourceMapping): ''' return list(self._structures.values()) + @property + def structures_inherited(self): + ''' + list(Structure): List of the inherited structures + ''' + return [s for s in self.structures if s.contract != self] + + @property + def structures_declared(self): + ''' + list(Structues): List of the structures declared within the contract (not inherited) + ''' + return [s for s in self.structures if s.contract == self] + def structures_as_dict(self): return self._structures @@ -87,6 +101,20 @@ class Contract(ChildSlither, SourceMapping): def enums(self): return list(self._enums.values()) + @property + def enums_inherited(self): + ''' + list(Enum): List of the inherited enums + ''' + return [e for e in self.enums if e.contract != self] + + @property + def enums_declared(self): + ''' + list(Enum): List of the enums declared within the contract (not inherited) + ''' + return [e for e in self.enums if e.contract == self] + def enums_as_dict(self): return self._enums @@ -104,6 +132,20 @@ class Contract(ChildSlither, SourceMapping): ''' return list(self._events.values()) + @property + def events_inherited(self): + ''' + list(Event): List of the inherited events + ''' + return [e for e in self.events if e.contract != self] + + @property + def events_declared(self): + ''' + list(Event): List of the events declared within the contract (not inherited) + ''' + return [e for e in self.events if e.contract == self] + def events_as_dict(self): return self._events @@ -149,6 +191,20 @@ class Contract(ChildSlither, SourceMapping): ''' return list(self._variables.values()) + @property + def state_variables_inherited(self): + ''' + list(StateVariable): List of the inherited state variables + ''' + return [s for s in self.state_variables if s.contract != self] + + @property + def state_variables_declared(self): + ''' + list(StateVariable): List of the state variables declared within the contract (not inherited) + ''' + return [s for s in self.state_variables if s.contract == self] + @property def slithir_variables(self): ''' @@ -173,17 +229,17 @@ class Contract(ChildSlither, SourceMapping): executed, following the c3 linearization Return None if there is no constructor. ''' - cst = self.constructor_not_inherited + cst = self.constructors_declared if cst: return cst for inherited_contract in self.inheritance: - cst = inherited_contract.constructor_not_inherited + cst = inherited_contract.constructors_declared if cst: return cst return None @property - def constructor_not_inherited(self): + def constructors_declared(self): return next((func for func in self.functions if func.is_constructor and func.contract_declarer == self), None) @property @@ -231,7 +287,7 @@ class Contract(ChildSlither, SourceMapping): return [f for f in self.functions if f.contract_declarer != self] @property - def functions_not_inherited(self): + def functions_declared(self): ''' list(Function): List of the functions defined within the contract (not inherited) ''' @@ -242,7 +298,7 @@ class Contract(ChildSlither, SourceMapping): ''' list(Functions): List of public and external functions ''' - return [f for f in self.functions if f.visibility in ['public', 'external']] + return [f for f in self.functions if f.visibility in ['public', 'external'] and not f.is_shadowed] @property def modifiers(self): @@ -262,7 +318,7 @@ class Contract(ChildSlither, SourceMapping): return [m for m in self.modifiers if m.contract_declarer != self] @property - def modifiers_not_inherited(self): + def modifiers_declared(self): ''' list(Modifier): List of the modifiers defined within the contract (not inherited) ''' @@ -283,11 +339,11 @@ class Contract(ChildSlither, SourceMapping): return self.functions_inherited + self.modifiers_inherited @property - def functions_and_modifiers_not_inherited(self): + def functions_and_modifiers_declared(self): ''' list(Function|Modifier): List of the functions and modifiers defined within the contract (not inherited) ''' - return self.functions_not_inherited + self.modifiers_not_inherited + return self.functions_declared + self.modifiers_declared def available_elements_from_inheritances(self, elements, getter_available): """ @@ -504,7 +560,7 @@ class Contract(ChildSlither, SourceMapping): list(core.Function) ''' - candidates = [c.functions_not_inherited for c in self.inheritance] + candidates = [c.functions_declared for c in self.inheritance] candidates = [candidate for sublist in candidates for candidate in sublist] return [f for f in candidates if f.full_name == function.full_name] diff --git a/slither/core/declarations/enum.py b/slither/core/declarations/enum.py index d04d0f6eb..ace9b2095 100644 --- a/slither/core/declarations/enum.py +++ b/slither/core/declarations/enum.py @@ -19,5 +19,13 @@ class Enum(ChildContract, SourceMapping): def values(self): return self._values + def is_declared_by(self, contract): + """ + Check if the element is declared by the contract + :param contract: + :return: + """ + return self.contract == contract + def __str__(self): return self.name diff --git a/slither/core/declarations/event.py b/slither/core/declarations/event.py index 29811f946..7d4eeeaf7 100644 --- a/slither/core/declarations/event.py +++ b/slither/core/declarations/event.py @@ -41,5 +41,13 @@ class Event(ChildContract, SourceMapping): def elems(self): return self._elems + def is_declared_by(self, contract): + """ + Check if the element is declared by the contract + :param contract: + :return: + """ + return self.contract == contract + def __str__(self): return self.name diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py index fa0542ea3..1c8a9103c 100644 --- a/slither/core/declarations/function.py +++ b/slither/core/declarations/function.py @@ -140,6 +140,14 @@ class Function(ChildContract, ChildInheritance, SourceMapping): def slither(self): return self.contract.slither + def is_declared_by(self, contract): + """ + Check if the element is declared by the contract + :param contract: + :return: + """ + return self.contract_declarer == contract + # endregion ################################################################################### ################################################################################### @@ -324,7 +332,7 @@ class Function(ChildContract, ChildInheritance, SourceMapping): included. """ # This is a list of contracts internally, so we convert it to a list of constructor functions. - return [c.constructor_not_inherited for c in self._explicit_base_constructor_calls if c.constructor_not_inherited] + return [c.constructors_declared for c in self._explicit_base_constructor_calls if c.constructors_declared] # endregion @@ -577,7 +585,7 @@ class Function(ChildContract, ChildInheritance, SourceMapping): list(core.Function) ''' - candidates = [c.functions_not_inherited for c in self.contract.inheritance] + candidates = [c.functions_declared for c in self.contract.inheritance] candidates = [candidate for sublist in candidates for candidate in sublist] return [f for f in candidates if f.full_name == self.full_name] diff --git a/slither/core/declarations/structure.py b/slither/core/declarations/structure.py index b11fb7e35..5d36ffe7c 100644 --- a/slither/core/declarations/structure.py +++ b/slither/core/declarations/structure.py @@ -23,5 +23,13 @@ class Structure(ChildContract, SourceMapping): def elems(self): return self._elems + def is_declared_by(self, contract): + """ + Check if the element is declared by the contract + :param contract: + :return: + """ + return self.contract == contract + def __str__(self): return self.name diff --git a/slither/core/variables/variable.py b/slither/core/variables/variable.py index e1476ed19..2f54db6e2 100644 --- a/slither/core/variables/variable.py +++ b/slither/core/variables/variable.py @@ -78,6 +78,14 @@ class Variable(SourceMapping): assert isinstance(t, (Type, list)) or t is None self._type = t + def is_declared_by(self, contract): + """ + Check if the element is declared by the contract + :param contract: + :return: + """ + return self.contract == contract + def __str__(self): return self._name diff --git a/slither/detectors/erc20/unindexed_event_parameters.py b/slither/detectors/erc20/unindexed_event_parameters.py index 71327f94e..dd10267f9 100644 --- a/slither/detectors/erc20/unindexed_event_parameters.py +++ b/slither/detectors/erc20/unindexed_event_parameters.py @@ -47,11 +47,7 @@ In this case, Transfer and Approval events should have the 'indexed' keyword on return results # Loop through all events to look for poor form. - for event in contract.events: - - # Only handle events which are declared in this contract. - if event.contract != contract: - continue + for event in contract.events_declared: # If this is transfer/approval events, expect the first two parameters to be indexed. if event.full_name in ["Transfer(address,address,uint256)", diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 50e0d52b2..9c95bde31 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -71,7 +71,7 @@ class ExternalFunction(AbstractDetector): for contract in function.contract.inheritance + [function.contract]: # Loop through the functions not inherited (explicitly defined in this contract). - for f in contract.functions_not_inherited: + for f in contract.functions_declared: # If it matches names, this is the base most function. if f.full_name == function.full_name: @@ -120,7 +120,7 @@ class ExternalFunction(AbstractDetector): continue # Next we'll want to loop through all functions defined directly in this contract. - for function in contract.functions_not_inherited: + for function in contract.functions_declared: # If the function is a constructor, or is public, we skip it. if function.is_constructor or function.visibility != "public": diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index b74696f34..b20382b83 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -69,13 +69,10 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 json['elements'] = [elem] results.append(json) - for struct in contract.structures: - if struct.contract != contract: - continue - + for struct in contract.structures_declared: if not self.is_cap_words(struct.name): - info = "Struct '{}.{}' ({}) is not in CapWords\n" - info = info.format(struct.contract.name, struct.name, struct.source_mapping_str) + info = "Struct '{}' ({}) is not in CapWords\n" + info = info.format(struct.canonical_name, struct.source_mapping_str) json = self.generate_json_result(info) elem = dict() @@ -85,10 +82,8 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 elem['source_mapping'] = struct.source_mapping json['elements'] = [elem] results.append(json) - for event in contract.events: - if event.contract != contract: - continue + for event in contract.events_declared: if not self.is_cap_words(event.name): info = "Event '{}' ({}) is not in CapWords\n" info = info.format(event.canonical_name, event.source_mapping_str) @@ -102,10 +97,7 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 json['elements'] = [elem] results.append(json) - for func in contract.functions: - if func.contract_declarer != contract: - continue - + for func in contract.functions_declared: if not self.is_mixed_case(func.name): info = "Function '{}' ({}) is not in mixedCase\n" info = info.format(func.canonical_name, func.source_mapping_str) @@ -139,10 +131,7 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 json['elements'] = [elem] results.append(json) - for var in contract.state_variables: - if var.contract != contract: - continue - + for var in contract.state_variables_declared: if self.should_avoid_name(var.name): if not self.is_upper_case_with_underscores(var.name): info = "Variable '{}' ({}) used l, O, I, which should not be used\n" @@ -193,10 +182,7 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 json['elements'] = [elem] results.append(json) - for enum in contract.enums: - if enum.contract != contract: - continue - + for enum in contract.enums_declared: if not self.is_cap_words(enum.name): info = "Enum '{}' ({}) is not in CapWords\n" info = info.format(enum.canonical_name, enum.source_mapping_str) @@ -210,11 +196,7 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 json['elements'] = [elem] results.append(json) - - for modifier in contract.modifiers: - if modifier.contract_declarer != contract: - continue - + for modifier in contract.modifiers_declared: if not self.is_mixed_case(modifier.name): info = "Modifier '{}' ({}) is not in mixedCase\n" info = info.format(modifier.canonical_name, @@ -229,5 +211,4 @@ Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.2 json['elements'] = [elem] results.append(json) - return results diff --git a/slither/detectors/reentrancy/reentrancy.py b/slither/detectors/reentrancy/reentrancy.py index 527e80273..be5f95dd9 100644 --- a/slither/detectors/reentrancy/reentrancy.py +++ b/slither/detectors/reentrancy/reentrancy.py @@ -181,7 +181,7 @@ class Reentrancy(AbstractDetector): def detect_reentrancy(self, contract): """ """ - for function in contract.functions_and_modifiers_not_inherited: + for function in contract.functions_and_modifiers_declared: if function.is_implemented: if self.KEY in function.context: continue diff --git a/slither/detectors/reentrancy/reentrancy_benign.py b/slither/detectors/reentrancy/reentrancy_benign.py index 7cecc3e64..7a6ee795e 100644 --- a/slither/detectors/reentrancy/reentrancy_benign.py +++ b/slither/detectors/reentrancy/reentrancy_benign.py @@ -45,7 +45,7 @@ Only report reentrancy that acts as a double call (see `reentrancy-eth`, `reentr def find_reentrancies(self): result = {} for contract in self.contracts: - for f in contract.functions_and_modifiers_not_inherited: + for f in contract.functions_and_modifiers_declared: for node in f.nodes: # dead code if not self.KEY in node.context: diff --git a/slither/detectors/reentrancy/reentrancy_eth.py b/slither/detectors/reentrancy/reentrancy_eth.py index 0ea1b9357..39b30a42d 100644 --- a/slither/detectors/reentrancy/reentrancy_eth.py +++ b/slither/detectors/reentrancy/reentrancy_eth.py @@ -47,7 +47,7 @@ Bob uses the re-entrancy bug to call `withdrawBalance` two times, and withdraw m def find_reentrancies(self): result = {} for contract in self.contracts: - for f in contract.functions_and_modifiers_not_inherited: + for f in contract.functions_and_modifiers_declared: for node in f.nodes: # dead code if not self.KEY in node.context: diff --git a/slither/detectors/reentrancy/reentrancy_read_before_write.py b/slither/detectors/reentrancy/reentrancy_read_before_write.py index 95c0e5012..80962bfab 100644 --- a/slither/detectors/reentrancy/reentrancy_read_before_write.py +++ b/slither/detectors/reentrancy/reentrancy_read_before_write.py @@ -46,7 +46,7 @@ Do not report reentrancies that involve ethers (see `reentrancy-eth`)''' def find_reentrancies(self): result = {} for contract in self.contracts: - for f in contract.functions_and_modifiers_not_inherited: + for f in contract.functions_and_modifiers_declared: for node in f.nodes: # dead code if not self.KEY in node.context: diff --git a/slither/detectors/shadowing/abstract.py b/slither/detectors/shadowing/abstract.py index cafdf96f2..15bb7e2c1 100644 --- a/slither/detectors/shadowing/abstract.py +++ b/slither/detectors/shadowing/abstract.py @@ -41,9 +41,9 @@ contract DerivedContract is BaseContract{ variables_fathers = [] for father in contract.inheritance: if all(not f.is_implemented for f in father.functions + father.modifiers): - variables_fathers += [v for v in father.variables if v.contract == father] + variables_fathers += father.state_variables_declared - for var in [v for v in contract.variables if v.contract == contract]: + for var in contract.state_variables_declared: shadow = [v for v in variables_fathers if v.name == var.name] if shadow: ret.append([var] + shadow) @@ -65,12 +65,10 @@ contract DerivedContract is BaseContract{ for all_variables in shadowing: shadow = all_variables[0] variables = all_variables[1:] - info = '{}.{} ({}) shadows:\n'.format(shadow.contract.name, - shadow.name, - shadow.source_mapping_str) + info = '{} ({}) shadows:\n'.format(shadow.canonical_name, + shadow.source_mapping_str) for var in variables: - info += "\t- {}.{} ({})\n".format(var.contract.name, - var.name, + info += "\t- {} ({})\n".format(var.canonical_name, var.source_mapping_str) json = self.generate_json_result(info) diff --git a/slither/detectors/shadowing/builtin_symbols.py b/slither/detectors/shadowing/builtin_symbols.py index d04f24d54..2cd4cff09 100644 --- a/slither/detectors/shadowing/builtin_symbols.py +++ b/slither/detectors/shadowing/builtin_symbols.py @@ -90,24 +90,20 @@ contract Bug { result = [] # Loop through all functions, modifiers, variables (state and local) to detect any built-in symbol keywords. - for function in contract.functions: - if function.contract_declarer == contract: - if self.is_builtin_symbol(function.name): - result.append((self.SHADOWING_FUNCTION, function, None)) - result += self.detect_builtin_shadowing_locals(function) - for modifier in contract.modifiers: - if modifier.contract_declarer == contract: - if self.is_builtin_symbol(modifier.name): - result.append((self.SHADOWING_MODIFIER, modifier, None)) - result += self.detect_builtin_shadowing_locals(modifier) - for variable in contract.variables: - if variable.contract == contract: - if self.is_builtin_symbol(variable.name): - result.append((self.SHADOWING_STATE_VARIABLE, variable, None)) - for event in contract.events: - if event.contract == contract: - if self.is_builtin_symbol(event.name): - result.append((self.SHADOWING_EVENT, event, None)) + for function in contract.functions_declared: + if self.is_builtin_symbol(function.name): + result.append((self.SHADOWING_FUNCTION, function, None)) + result += self.detect_builtin_shadowing_locals(function) + for modifier in contract.modifiers_declared: + if self.is_builtin_symbol(modifier.name): + result.append((self.SHADOWING_MODIFIER, modifier, None)) + result += self.detect_builtin_shadowing_locals(modifier) + for variable in contract.state_variables_declared: + if self.is_builtin_symbol(variable.name): + result.append((self.SHADOWING_STATE_VARIABLE, variable, None)) + for event in contract.events_declared: + if self.is_builtin_symbol(event.name): + result.append((self.SHADOWING_EVENT, event, None)) return result diff --git a/slither/detectors/shadowing/local.py b/slither/detectors/shadowing/local.py index aadc637b9..0ba24811f 100644 --- a/slither/detectors/shadowing/local.py +++ b/slither/detectors/shadowing/local.py @@ -67,20 +67,20 @@ contract Bug { overshadowed = [] for scope_contract in [contract] + contract.inheritance: # Check functions - for scope_function in scope_contract.functions: - if variable.name == scope_function.name and scope_function.contract_declarer == scope_contract: + for scope_function in scope_contract.functions_declared: + if variable.name == scope_function.name: overshadowed.append((self.OVERSHADOWED_FUNCTION, scope_contract.name, scope_function)) # Check modifiers - for scope_modifier in scope_contract.modifiers: - if variable.name == scope_modifier.name and scope_modifier.contract_declarer == scope_contract: + for scope_modifier in scope_contract.modifiers_declared: + if variable.name == scope_modifier.name: overshadowed.append((self.OVERSHADOWED_MODIFIER, scope_contract.name, scope_modifier)) # Check events - for scope_event in scope_contract.events: - if variable.name == scope_event.name and scope_event.contract == scope_contract: + for scope_event in scope_contract.events_declared: + if variable.name == scope_event.name: overshadowed.append((self.OVERSHADOWED_EVENT, scope_contract.name, scope_event)) # Check state variables - for scope_state_variable in scope_contract.variables: - if variable.name == scope_state_variable.name and scope_state_variable.contract == scope_contract: + for scope_state_variable in scope_contract.state_variables_declared: + if variable.name == scope_state_variable.name: overshadowed.append((self.OVERSHADOWED_STATE_VARIABLE, scope_contract.name, scope_state_variable)) # If we have found any overshadowed objects, we'll want to add it to our result list. diff --git a/slither/detectors/shadowing/state.py b/slither/detectors/shadowing/state.py index 75ba66132..41da711f3 100644 --- a/slither/detectors/shadowing/state.py +++ b/slither/detectors/shadowing/state.py @@ -53,9 +53,9 @@ contract DerivedContract is BaseContract{ variables_fathers = [] for father in contract.inheritance: if any(f.is_implemented for f in father.functions + father.modifiers): - variables_fathers += [v for v in father.variables if v.contract == father] + variables_fathers += father.state_variables_declared - for var in [v for v in contract.variables if v.contract == contract]: + for var in contract.state_variables_declared: shadow = [v for v in variables_fathers if v.name == var.name] if shadow: ret.append([var] + shadow) diff --git a/slither/detectors/statements/deprecated_calls.py b/slither/detectors/statements/deprecated_calls.py index 6d0566549..c18843125 100644 --- a/slither/detectors/statements/deprecated_calls.py +++ b/slither/detectors/statements/deprecated_calls.py @@ -111,20 +111,14 @@ contract ContractWithDeprecatedReferences { list of tuple: (state_variable | node, (detecting_signature, original_text, recommended_text))""" results = [] - for state_variable in contract.variables: - if state_variable.contract != contract: - continue + for state_variable in contract.state_variables_declared: if state_variable.expression: deprecated_results = self.detect_deprecation_in_expression(state_variable.expression) if deprecated_results: results.append((state_variable, deprecated_results)) # Loop through all functions + modifiers in this contract. - for function in contract.functions + contract.modifiers: - # We should only look for functions declared directly in this contract (not in a base contract). - if function.contract != contract: - continue - + for function in contract.functions_and_modifiers_declared: # Loop through each node in this function. for node in function.nodes: # Detect deprecated references in the node. diff --git a/slither/printers/inheritance/inheritance_graph.py b/slither/printers/inheritance/inheritance_graph.py index 75c60b5e1..73ee72af8 100644 --- a/slither/printers/inheritance/inheritance_graph.py +++ b/slither/printers/inheritance/inheritance_graph.py @@ -91,8 +91,8 @@ class PrinterInheritanceGraph(AbstractPrinter): indirect_shadows = detect_c3_function_shadowing(contract) if indirect_shadows: for collision_set in sorted(indirect_shadows, key=lambda x: x[0][1].name): - winner = collision_set[-1][1].contract.name - collision_steps = [colliding_function.contract.name for _, colliding_function in collision_set] + winner = collision_set[-1][1].contract_declarer.name + collision_steps = [colliding_function.contract_declarer.name for _, colliding_function in collision_set] collision_steps = ', '.join(collision_steps) result.append(f"'{collision_set[0][1].full_name}' collides in inherited contracts {collision_steps} where {winner} is chosen.") return '\n'.join(result) @@ -127,12 +127,12 @@ class PrinterInheritanceGraph(AbstractPrinter): modifiers = ''.join(modifiers) # Public variables - public_variables = [self._get_pattern_var(v, contract) for v in contract.variables if - v.contract == contract and v.visibility in visibilities] + public_variables = [self._get_pattern_var(v, contract) for v in contract.state_variables_declared + if v.visibility in visibilities] public_variables = ''.join(public_variables) - private_variables = [self._get_pattern_var(v, contract) for v in contract.variables if - v.contract == contract and v.visibility not in visibilities] + private_variables = [self._get_pattern_var(v, contract) for v in contract.state_variables_declared + if v.visibility not in visibilities] private_variables = ''.join(private_variables) # Obtain any indirect shadowing information for this node. diff --git a/slither/printers/summary/data_depenency.py b/slither/printers/summary/data_depenency.py index d64a0f638..c8fb38277 100644 --- a/slither/printers/summary/data_depenency.py +++ b/slither/printers/summary/data_depenency.py @@ -35,7 +35,7 @@ class DataDependency(AbstractPrinter): txt += str(table) txt += "\n" - for f in c.functions_and_modifiers_not_inherited: + for f in c.functions_and_modifiers_declared: txt += "\nFunction %s\n"%f.full_name table = PrettyTable(['Variable', 'Dependencies']) for v in f.variables: diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 51993b2c6..2bff4b33d 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -658,7 +658,10 @@ def look_for_library(contract, ir, node, using_for, t): return None def convert_to_library(ir, node, using_for): - contract = node.function.contract + # We use contract_declarer, because Solidity resolve the library + # before resolving the inheritance. + # Though we could use .contract as libraries cannot be shadowed + contract = node.function.contract_declarer t = ir.destination.type if t in using_for: diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 27de7b4b1..c70e449c3 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -471,11 +471,10 @@ class ContractSolc04(Contract): all_ssa_state_variables_instances = dict() for contract in self.inheritance: - for v in contract.variables: - if v.contract == contract: - new_var = StateIRVariable(v) - all_ssa_state_variables_instances[v.canonical_name] = new_var - self._initial_state_variables.append(new_var) + for v in contract.state_variables_declared: + new_var = StateIRVariable(v) + all_ssa_state_variables_instances[v.canonical_name] = new_var + self._initial_state_variables.append(new_var) for v in self.variables: if v.contract == self: diff --git a/slither/utils/inheritance_analysis.py b/slither/utils/inheritance_analysis.py index 0c0f7c6da..3caa06b74 100644 --- a/slither/utils/inheritance_analysis.py +++ b/slither/utils/inheritance_analysis.py @@ -61,7 +61,7 @@ def detect_direct_function_shadowing(contract): function (could have provided it through inheritance, does not need to directly define it). -overshadowed_function is the function definition which is overshadowed by the provided contract's definition. """ - functions_declared = {function.full_name: function for function in contract.functions_and_modifiers_not_inherited} + functions_declared = {function.full_name: function for function in contract.functions_and_modifiers_declared} results = {} for base_contract in reversed(contract.immediate_inheritance): for base_function in base_contract.functions_and_modifiers: @@ -128,8 +128,7 @@ def detect_state_variable_shadowing(contracts): """ results = set() for contract in contracts: - variables_declared = {variable.name: variable for variable in contract.variables - if variable.contract == contract} + variables_declared = {variable.name: variable for variable in contract.state_variables_declared} for immediate_base_contract in contract.immediate_inheritance: for variable in immediate_base_contract.variables: if variable.name in variables_declared: diff --git a/utils/possible_paths/possible_paths.py b/utils/possible_paths/possible_paths.py index 0d9976e08..e638b00ad 100644 --- a/utils/possible_paths/possible_paths.py +++ b/utils/possible_paths/possible_paths.py @@ -67,7 +67,7 @@ def all_function_definitions(function): :return: Returns a list composed of the provided function definition and any base definitions. """ return [function] + [f for c in function.contract.inheritance - for f in c.functions_and_modifiers_not_inherited + for f in c.functions_and_modifiers_declared if f.full_name == function.full_name] @@ -84,7 +84,7 @@ def __find_target_paths(slither, target_function, current_path=[]): # Look through all functions for contract in slither.contracts: - for function in contract.functions_and_modifiers_not_inherited: + for function in contract.functions_and_modifiers_declared: # If the function is already in our path, skip it. if function in current_path: From d20b226da7f69b30323220a89c1abd30889d1751 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 29 Apr 2019 16:13:46 +0100 Subject: [PATCH 08/11] Fix bugs in call graph printer --- slither/printers/call/call_graph.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/slither/printers/call/call_graph.py b/slither/printers/call/call_graph.py index cf6776e69..4c9b9e385 100644 --- a/slither/printers/call/call_graph.py +++ b/slither/printers/call/call_graph.py @@ -71,7 +71,7 @@ class PrinterCallGraph(AbstractPrinter): for contract in all_contracts: render_internal_calls += self._render_internal_calls(contract, contract_functions, contract_calls) - render_solidity_calls = '' #self._render_solidity_calls(solidity_functions, solidity_calls) + render_solidity_calls = self._render_solidity_calls(solidity_functions, solidity_calls) render_external_calls = self._render_external_calls(external_calls) @@ -110,7 +110,6 @@ class PrinterCallGraph(AbstractPrinter): # add variable as node to respective contract if isinstance(external_function, (Variable)): - return contract_functions[external_contract].add(_node( _function_node(external_contract, external_function), external_function.name From 5a9db1d11862c69c99963c1c27aec1cda89b2f9a Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 29 Apr 2019 16:40:39 +0100 Subject: [PATCH 09/11] Fix inheritance graph --- slither/utils/inheritance_analysis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/utils/inheritance_analysis.py b/slither/utils/inheritance_analysis.py index 3caa06b74..013de531e 100644 --- a/slither/utils/inheritance_analysis.py +++ b/slither/utils/inheritance_analysis.py @@ -19,7 +19,7 @@ def detect_c3_function_shadowing(contract): for i in range(0, len(contract.immediate_inheritance) - 1): inherited_contract1 = contract.immediate_inheritance[i] - for function1 in inherited_contract1.functions_and_modifiers: + for function1 in inherited_contract1.functions_and_modifiers_declared: # If this function has already be handled or is unimplemented, we skip it if function1.full_name in results or function1.is_constructor or not function1.is_implemented: continue From ea9e46b313b25a60a45f8c3e90b97fc6f4185d38 Mon Sep 17 00:00:00 2001 From: Josselin Date: Mon, 29 Apr 2019 17:06:32 +0100 Subject: [PATCH 10/11] Fix incorrect is_declared_by func --- slither/core/variables/state_variable.py | 7 +++++++ slither/core/variables/variable.py | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/slither/core/variables/state_variable.py b/slither/core/variables/state_variable.py index 86c21e30e..8fee502bb 100644 --- a/slither/core/variables/state_variable.py +++ b/slither/core/variables/state_variable.py @@ -3,6 +3,13 @@ from slither.core.children.child_contract import ChildContract class StateVariable(ChildContract, Variable): + def is_declared_by(self, contract): + """ + Check if the element is declared by the contract + :param contract: + :return: + """ + return self.contract == contract @property def canonical_name(self): diff --git a/slither/core/variables/variable.py b/slither/core/variables/variable.py index 2f54db6e2..8b37c6da1 100644 --- a/slither/core/variables/variable.py +++ b/slither/core/variables/variable.py @@ -78,13 +78,6 @@ class Variable(SourceMapping): assert isinstance(t, (Type, list)) or t is None self._type = t - def is_declared_by(self, contract): - """ - Check if the element is declared by the contract - :param contract: - :return: - """ - return self.contract == contract def __str__(self): return self._name From 6a473c7e87f510f016dfadb1f5ba7f5b05940db8 Mon Sep 17 00:00:00 2001 From: Josselin Date: Wed, 15 May 2019 10:15:30 +0100 Subject: [PATCH 11/11] Minor --- slither/detectors/abstract_detector.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/detectors/abstract_detector.py b/slither/detectors/abstract_detector.py index ad22ae092..39651c12a 100644 --- a/slither/detectors/abstract_detector.py +++ b/slither/detectors/abstract_detector.py @@ -160,8 +160,8 @@ class AbstractDetector(metaclass=abc.ABCMeta): def _create_parent_element(element): from slither.core.children.child_contract import ChildContract from slither.core.children.child_function import ChildFunction - from slither.core.declarations import Function - if isinstance(element, Function): + from slither.core.children.child_inheritance import ChildInheritance + if isinstance(element, ChildInheritance): if element.contract_declarer: contract = {'elements': []} AbstractDetector.add_contract_to_json(element.contract_declarer, contract)