diff --git a/slither/detectors/shadowing/shadowing_functions.py b/slither/detectors/shadowing/shadowing_functions.py deleted file mode 100644 index b5d1f8444c..0000000000 --- a/slither/detectors/shadowing/shadowing_functions.py +++ /dev/null @@ -1,59 +0,0 @@ -""" - Module detecting shadowing of functions - It is more useful as summary printer than as vuln detection -""" - -from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification - - -class ShadowingFunctionsDetection(AbstractDetector): - """ - Functions shadowing detection - """ - - vuln_name = "ShadowingFunctionContract" - - ARGUMENT = 'shadowing-function' - HELP = 'Function Shadowing' - IMPACT = DetectorClassification.LOW - CONFIDENCE = DetectorClassification.HIGH - - # This detector is not meant to be called as a generic detector - # It's only used by inheritances printers - WIKI = 'undefined' - WIKI_TITLE = 'undefined' - WIKI_DESCRIPTION = 'undefined' - WIKI_EXPLOIT_SCENARIO = 'undefined' - WIKI_RECOMMENDATION = 'undefined' - - - def detect_shadowing(self, contract): - functions_declared = set([x.full_name for x in contract.functions]) - ret = {} - for father in contract.inheritance: - functions_declared_father = ([x.full_name for x in father.functions]) - inter = functions_declared.intersection(functions_declared_father) - if inter: - ret[father] = inter - return ret - - def detect(self): - """ detect shadowing - - recursively visit the calls - returns: - list: {'vuln', 'filename,'contract','func', 'shadow'} - - """ - results = [] - for c in self.contracts: - shadowing = self.detect_shadowing(c) - if shadowing: - for contract, funcs in shadowing.items(): - results.append({'vuln': self.vuln_name, - 'filename': self.filename, - 'contractShadower': c.name, - 'contract': contract.name, - 'functions': list(funcs)}) - - return results diff --git a/slither/printers/inheritance/inheritance_graph.py b/slither/printers/inheritance/inheritance_graph.py index ed1c241735..5b9cdbe4c2 100644 --- a/slither/printers/inheritance/inheritance_graph.py +++ b/slither/printers/inheritance/inheritance_graph.py @@ -7,8 +7,12 @@ """ from slither.core.declarations.contract import Contract -from slither.detectors.shadowing.shadowing_functions import ShadowingFunctionsDetection +from slither.core.solidity_types.user_defined_type import UserDefinedType from slither.printers.abstract_printer import AbstractPrinter +from slither.utils.inheritance_analysis import (detect_c3_function_shadowing, + detect_function_shadowing, + detect_state_variable_shadowing) + class PrinterInheritanceGraph(AbstractPrinter): ARGUMENT = 'inheritance-graph' @@ -20,23 +24,37 @@ def __init__(self, slither, logger): inheritance = [x.inheritance for x in slither.contracts] self.inheritance = set([item for sublist in inheritance for item in sublist]) - shadow = ShadowingFunctionsDetection(slither, None) - ret = shadow.detect() - functions_shadowed = {} - for s in ret: - if s['contractShadower'] not in functions_shadowed: - functions_shadowed[s['contractShadower']] = [] - functions_shadowed[s['contractShadower']] += s['functions'] - self.functions_shadowed = functions_shadowed + # Create a lookup of direct shadowing instances. + self.direct_overshadowing_functions = {} + shadows = detect_function_shadowing(slither.contracts, True, False) + for overshadowing_instance in shadows: + overshadowing_function = overshadowing_instance[2] + + # Add overshadowing function entry. + if overshadowing_function not in self.direct_overshadowing_functions: + self.direct_overshadowing_functions[overshadowing_function] = set() + self.direct_overshadowing_functions[overshadowing_function].add(overshadowing_instance) + + # Create a lookup of shadowing state variables. + # Format: { colliding_variable : set([colliding_variables]) } + self.overshadowing_state_variables = {} + shadows = detect_state_variable_shadowing(slither.contracts) + for overshadowing_instance in shadows: + overshadowing_state_var = overshadowing_instance[1] + overshadowed_state_var = overshadowing_instance[3] + + # Add overshadowing variable entry. + if overshadowing_state_var not in self.overshadowing_state_variables: + self.overshadowing_state_variables[overshadowing_state_var] = set() + self.overshadowing_state_variables[overshadowing_state_var].add(overshadowed_state_var) def _get_pattern_func(self, func, contract): # Html pattern, each line is a row in a table func_name = func.full_name pattern = ' %s' pattern_shadow = ' %s' - if contract.name in self.functions_shadowed: - if func_name in self.functions_shadowed[contract.name]: - return pattern_shadow % func_name + if func in self.direct_overshadowing_functions: + return pattern_shadow % func_name return pattern % func_name def _get_pattern_var(self, var, contract): @@ -44,11 +62,38 @@ def _get_pattern_var(self, var, contract): var_name = var.name pattern = ' %s' pattern_contract = ' %s (%s)' - # pattern_arrow = ' %s' - if isinstance(var.type, Contract): - return pattern_contract % (var_name, str(var.type)) - # return pattern_arrow%(self._get_port_id(var, contract), var_name) - return pattern % var_name + pattern_shadow = ' %s' + pattern_contract_shadow = ' %s (%s)' + + if isinstance(var.type, UserDefinedType) and isinstance(var.type.type, Contract): + if var in self.overshadowing_state_variables: + return pattern_contract_shadow % (var_name, var.type.type.name) + else: + return pattern_contract % (var_name, var.type.type.name) + else: + if var in self.overshadowing_state_variables: + return pattern_shadow % var_name + else: + return pattern % var_name + + @staticmethod + def _get_indirect_shadowing_information(contract): + """ + Obtain a string that describes variable shadowing for the given variable. None if no shadowing exists. + :param var: The variable to collect shadowing information for. + :param contract: The contract in which this variable is being analyzed. + :return: Returns a string describing variable shadowing for the given variable. None if no shadowing exists. + """ + # If this variable is an overshadowing variable, we'll want to return information describing it. + result = [] + 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] + collision_steps = ', '.join(collision_steps) + result.append(f"'{collision_set[0][1].name}' collides in inherited contracts {collision_steps} where {winner} wins.") + return '\n'.join(result) def _get_port_id(self, var, contract): return "%s%s" % (var.name, contract.name) @@ -58,9 +103,13 @@ def _summary(self, contract): Build summary using HTML """ ret = '' - # Add arrows - for i in contract.immediate_inheritance: - ret += '%s -> %s;\n' % (contract.name, i) + + # Add arrows (number them if there is more than one path so we know order of declaration for inheritance). + if len(contract.immediate_inheritance) == 1: + ret += '%s -> %s;\n' % (contract.name, contract.immediate_inheritance[0]) + else: + for i in range(0, len(contract.immediate_inheritance)): + ret += '%s -> %s [ label="%s" ];\n' % (contract.name, contract.immediate_inheritance[i], i + 1) # Functions visibilities = ['public', 'external'] @@ -70,18 +119,23 @@ def _summary(self, contract): 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] private_functions = ''.join(private_functions) + # Modifiers modifiers = [self._get_pattern_func(m, contract) for m in contract.modifiers if m.contract == contract] modifiers = ''.join(modifiers) + # Public variables public_variables = [self._get_pattern_var(v, contract) for v in contract.variables if - v.visibility in visibilities] + v.contract == contract and v.visibility in visibilities] public_variables = ''.join(public_variables) private_variables = [self._get_pattern_var(v, contract) for v in contract.variables if - not v.visibility in visibilities] + v.contract == contract and v.visibility not in visibilities] private_variables = ''.join(private_variables) + # Obtain any indirect shadowing information for this node. + indirect_shadowing_information = self._get_indirect_shadowing_information(contract) + # Build the node label ret += '%s[shape="box"' % contract.name ret += 'label=< ' @@ -101,6 +155,9 @@ def _summary(self, contract): if private_variables: ret += '' ret += '%s' % private_variables + + if indirect_shadowing_information: + ret += '' % indirect_shadowing_information.replace('\n', '
') ret += '
Private Variables:

%s
>];\n' return ret @@ -118,7 +175,7 @@ def output(self, filename): info = 'Inheritance Graph: ' + filename self.info(info) with open(filename, 'w', encoding='utf8') as f: - f.write('digraph{\n') + f.write('digraph "" {\n') for c in self.contracts: f.write(self._summary(c)) f.write('}') diff --git a/slither/utils/inheritance_analysis.py b/slither/utils/inheritance_analysis.py new file mode 100644 index 0000000000..de05b246f4 --- /dev/null +++ b/slither/utils/inheritance_analysis.py @@ -0,0 +1,137 @@ +""" +Detects various properties of inheritance in provided contracts. +""" + + +def detect_c3_function_shadowing(contract): + """ + Detects and obtains functions which are indirectly shadowed via multiple inheritance by C3 linearization + properties, despite not directly inheriting from each other. + + :param contract: The contract to check for potential C3 linearization shadowing within. + :return: A list of list of tuples: (contract, function), where each inner list describes colliding functions. + The later elements in the inner list overshadow the earlier ones. The contract-function pair's function does not + need to be defined in its paired contract, it may have been inherited within it. + """ + + # Loop through all contracts, and all underlying functions. + results = {} + for i in range(0, len(contract.immediate_inheritance) - 1): + inherited_contract1 = contract.immediate_inheritance[i] + + for function1 in inherited_contract1.functions_and_modifiers: + # 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 + + # Define our list of function instances which overshadow each other. + functions_matching = [(inherited_contract1, function1)] + already_processed = set([function1]) + + # Loop again through other contracts and functions to compare to. + for x in range(i + 1, len(contract.immediate_inheritance)): + inherited_contract2 = contract.immediate_inheritance[x] + + # Loop for each function in this contract + for function2 in inherited_contract2.functions_and_modifiers: + # Skip this function if it is the last function that was shadowed. + if function2 in already_processed or function2.is_constructor or not function2.is_implemented: + continue + + # If this function does have the same full name, it is shadowing through C3 linearization. + if function1.full_name == function2.full_name: + functions_matching.append((inherited_contract2, function2)) + already_processed.add(function2) + + # If we have more than one definition matching the same signature, we add it to the results. + if len(functions_matching) > 1: + results[function1.full_name] = functions_matching + + return list(results.values()) + + +def detect_direct_function_shadowing(contract): + """ + Detects and obtains functions which are shadowed immediately by the provided ancestor contract. + :param contract: The ancestor contract which we check for function shadowing within. + :return: A list of tuples (overshadowing_function, overshadowed_immediate_base_contract, overshadowed_function) + -overshadowing_function is the function defined within the provided contract that overshadows another + definition. + -overshadowed_immediate_base_contract is the immediate inherited-from contract that provided the shadowed + 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} + results = {} + for base_contract in reversed(contract.immediate_inheritance): + for base_function in base_contract.functions_and_modifiers: + + # We already found the most immediate shadowed definition for this function, skip to the next. + if base_function.full_name in results: + continue + + # If this function is implemented and it collides with a definition in our immediate contract, we add + # it to our results. + if base_function.is_implemented and base_function.full_name in functions_declared: + results[base_function.full_name] = (functions_declared[base_function.full_name], base_contract, base_function) + + return list(results.values()) + + +def detect_function_shadowing(contracts, direct_shadowing=True, indirect_shadowing=True): + """ + Detects all overshadowing and overshadowed functions in the provided contracts. + :param contracts: The contracts to detect shadowing within. + :param direct_shadowing: Include results from direct inheritance/inheritance ancestry. + :param indirect_shadowing: Include results from indirect inheritance collisions as a result of multiple + inheritance/c3 linearization. + :return: Returns a set of tuples(contract_scope, overshadowing_contract, overshadowing_function, + overshadowed_contract, overshadowed_function), where: + -The contract_scope defines where the detection of shadowing is most immediately found. + -For each contract-function pair, contract is the first contract where the function is seen, while the function + refers to the actual definition. The function does not need to be defined in the contract (could be inherited). + """ + results = set() + for contract in contracts: + + # Detect immediate inheritance shadowing. + if direct_shadowing: + shadows = detect_direct_function_shadowing(contract) + for (overshadowing_function, overshadowed_base_contract, overshadowed_function) in shadows: + results.add((contract, contract, overshadowing_function, overshadowed_base_contract, + overshadowed_function)) + + # Detect c3 linearization shadowing (multi inheritance shadowing). + if indirect_shadowing: + shadows = detect_c3_function_shadowing(contract) + for colliding_functions in shadows: + for x in range(0, len(colliding_functions) - 1): + 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: + results.add((contract, colliding_functions[y][0], colliding_functions[y][1], + colliding_functions[x][0], colliding_functions[x][1])) + + return results + + +def detect_state_variable_shadowing(contracts): + """ + Detects all overshadowing and overshadowed state variables in the provided contracts. + :param contracts: The contracts to detect shadowing within. + :return: Returns a set of tuples (overshadowing_contract, overshadowing_state_var, overshadowed_contract, + overshadowed_state_var). + The contract-variable pair's variable does not need to be defined in its paired contract, it may have been + inherited. The contracts are simply included to denote the immediate inheritance path from which the shadowed + variable originates. + """ + results = set() + for contract in contracts: + variables_declared = {variable.name: variable for variable in contract.variables + if variable.contract == contract} + for immediate_base_contract in contract.immediate_inheritance: + for variable in immediate_base_contract.variables: + if variable.name in variables_declared: + results.add((contract, variables_declared[variable.name], immediate_base_contract, variable)) + return results diff --git a/tests/inheritance_graph.sol b/tests/inheritance_graph.sol new file mode 100644 index 0000000000..6707775833 --- /dev/null +++ b/tests/inheritance_graph.sol @@ -0,0 +1,100 @@ +contract TestContractVar { + +} + +contract A { + uint public public_var = 1; + uint internal private_var = 1; + TestContractVar public public_contract; + TestContractVar internal private_contract; + + uint public shadowed_public_var = 1; + uint internal shadowed_private_var = 1; + TestContractVar public shadowed_public_contract; + TestContractVar internal shadowed_private_contract; + + function getValue() public pure returns (uint) { + return 0; + } + function notRedefined() public returns (uint) { + return getValue(); + } + + modifier testModifier { + assert(true); + _; + } + function testFunction() testModifier public returns (uint) { + return 0; + } +} + +contract B is A { + // This function overshadows A directly, and overshadows C indirectly (via 'G'->'D') + function getValue() public pure returns (uint) { + return 1; + } +} + +contract Good is A, B { + +} + +contract C is A { + + // This function overshadows A directly, and overshadows B indirectly (via 'G') + function getValue() public pure returns (uint) { + return super.getValue() + 1; + } +} + +contract D is B { + // This should overshadow A's definitions. + uint public shadowed_public_var = 2; + uint internal shadowed_private_var = 2; + TestContractVar public shadowed_public_contract; + TestContractVar internal shadowed_private_contract; +} + +contract E { + // Variables cannot indirectly shadow, so this should not be counted. + uint public public_var = 2; + uint internal private_var = 2; + TestContractVar public public_contract; + TestContractVar internal private_contract; + + // This should overshadow A's definition indirectly (via 'G'). + modifier testModifier { + assert(false); + _; + } +} + +contract F is B { + // This should overshadow A's definitions. + uint public shadowed_public_var = 2; + uint internal shadowed_private_var = 2; + TestContractVar public shadowed_public_contract; + TestContractVar internal shadowed_private_contract; + + // This should overshadow B's definition directly, as well as B's and C's indirectly (via 'G') + // (graph only outputs directly if both, so B direct and C indirect should be reported). + function getValue() public pure returns (uint) { + return 1; + } + + // This should indirectly shadow definition in A directly, and E indirectly (via 'G') + modifier testModifier { + assert(false); + _; + } +} + +contract G is B, C, D, E, F { + // This should overshadow definitions in A, D, and F + uint public shadowed_public_var = 3; + uint internal shadowed_private_var = 3; + TestContractVar public shadowed_public_contract; + + // This contract's multiple inheritance chain should cause indirect shadowing (c3 linearization shadowing). +}