Fix shadowing-function detector and enhance inheritance-graph

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,35 +24,76 @@ 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 = '<TR><TD align="left">    %s</TD></TR>'
         pattern_shadow = '<TR><TD align="left"><font color="#FFA500">    %s</font></TD></TR>'
-        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):
         # Html pattern, each line is a row in a table
         var_name = var.name
         pattern = '<TR><TD align="left">    %s</TD></TR>'
         pattern_contract = '<TR><TD align="left">    %s<font color="blue" POINT-SIZE="10"> (%s)</font></TD></TR>'
-        # pattern_arrow  = '<TR><TD align="left" PORT="%s"><font color="blue">    %s</font></TD></TR>'
-        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 = '<TR><TD align="left"><font color="red">    %s</font></TD></TR>'
+        pattern_contract_shadow = '<TR><TD align="left"><font color="red">    %s</font><font color="blue" POINT-SIZE="10"> (%s)</font></TD></TR>'
+
+        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=< <TABLE border="0">'
@@ -101,6 +155,9 @@ def _summary(self, contract):
         if private_variables:
             ret += '<TR><TD align="left"><I>Private Variables:</I></TD></TR>'
             ret += '%s' % private_variables
+
+        if indirect_shadowing_information:
+            ret += '<TR><TD><BR/></TD></TR><TR><TD align="left" border="1"><font color="gray" point-size="10">%s</font></TD></TR>' % indirect_shadowing_information.replace('\n', '<BR/>')
         ret += '</TABLE> >];\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('}')

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