diff --git a/slither/printers/inheritance/inheritance_graph.py b/slither/printers/inheritance/inheritance_graph.py index 8cfbf2a36..e8c9d2fba 100644 --- a/slither/printers/inheritance/inheritance_graph.py +++ b/slither/printers/inheritance/inheritance_graph.py @@ -21,14 +21,22 @@ class PrinterInheritanceGraph(AbstractPrinter): inheritance = [x.inheritance for x in slither.contracts] self.inheritance = set([item for sublist in inheritance for item in sublist]) - # Obtain functions shadowed through direct inheritance lines. - shadows = InheritanceAnalysis.detect_function_shadowing(slither.contracts) + # Obtain functions shadowed through direct/indirect inheritance. self.overshadowed_functions = set() self.overshadowing_functions = set() + shadows = InheritanceAnalysis.detect_function_shadowing(slither.contracts) for _, overshadowing_function, _, overshadowed_function in shadows: self.overshadowing_functions.add(overshadowing_function) self.overshadowed_functions.add(overshadowed_function) + # Obtain state variables shadowed through direct inheritance. + self.overshadowed_state_variables = set() + self.overshadowing_state_variables = set() + shadows = InheritanceAnalysis.detect_state_variable_shadowing(slither.contracts) + for _, overshadowing_state_var, _, overshadowed_state_var in shadows: + self.overshadowing_state_variables.add(overshadowing_state_var) + self.overshadowed_state_variables.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 diff --git a/slither/utils/inheritance_analysis.py b/slither/utils/inheritance_analysis.py index 95d15879f..3d542c83d 100644 --- a/slither/utils/inheritance_analysis.py +++ b/slither/utils/inheritance_analysis.py @@ -85,9 +85,11 @@ class InheritanceAnalysis: :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(overshadowing_contract, overshadowing_function, overshadowed_contract, overshadowed_function). + :return: Returns a set of tuples(overshadowing_contract, overshadowing_function, overshadowed_contract, + overshadowed_function). The contract-function pair's function does not need to be defined in its paired contract, it may have been - inherited within it. + inherited within it. The contract is simply included to denote the immediate inheritance path from which the + shadowed function originates. """ results = set() for contract in contracts: @@ -107,3 +109,24 @@ class InheritanceAnalysis: colliding_functions[i][0], colliding_functions[i][1])) return results + + @staticmethod + 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 000000000..a17eaaa83 --- /dev/null +++ b/tests/inheritance_graph.sol @@ -0,0 +1,53 @@ +contract A { + function getValue() public pure returns (uint) { + // This function should be overshadowed directly by B, C, and indirectly by B (via 'Good') + return 0; + } + function notRedefined() public returns (uint) { + return getValue(); + } + + modifier testModifier { + // This is redefined in E. + assert(true); + _; + } + function testFunction() testModifier public returns (uint) { + return 0; + } +} + +contract B is A { + // This function should not be marked as overshadowed (although C overshadows it, D further overshadows it, and D + // derives from B, so it neutralizes any overshadowing for this contract). + function getValue() public pure returns (uint) { + return 1; + } +} + +contract Good is A, B{ + +} + +contract C is A { + function getValue() public pure returns (uint) { + // This function should be marked as overshadowed indirectly by D (via 'F') + return super.getValue() + 1; + } +} + +contract D is B { + // This should use B's getValue() to overshadow C's definition indirectly (via 'F'). +} + +contract E { + modifier testModifier { + // This should indirectly shadow A's definition (via 'F') + assert(false); + _; + } +} + +contract F is B, C, D, E { + // This multiple inheritance chain should cause indirect shadowing (c3 linearization shadowing). +}