Perform cross-contract taint analysis from diff of two upgrade versions

slither/utils/upgradeability.py

@@ -1,4 +1,4 @@
-from typing import Optional, Tuple, List, Union
+from typing import Optional, Tuple, List, Union, TypedDict
 from slither.core.declarations import (
     Contract,
     Structure,
@@ -17,12 +17,15 @@
 from slither.core.variables.local_variable import LocalVariable
 from slither.core.variables.local_variable_init_from_tuple import LocalVariableInitFromTuple
 from slither.core.variables.state_variable import StateVariable
+from slither.slithir.variables import TemporaryVariable
 from slither.analyses.data_dependency.data_dependency import get_dependencies
 from slither.core.variables.variable import Variable
-from slither.core.expressions.literal import Literal
-from slither.core.expressions.identifier import Identifier
-from slither.core.expressions.call_expression import CallExpression
-from slither.core.expressions.assignment_operation import AssignmentOperation
+from slither.core.expressions import (
+    Literal,
+    Identifier,
+    CallExpression,
+    AssignmentOperation,
+)
 from slither.core.cfg.node import Node, NodeType
 from slither.slithir.operations import (
     Operation,
@@ -61,11 +64,23 @@
 from slither.tools.read_storage.read_storage import SlotInfo, SlitherReadStorage
 
 
+class TaintedExternalContract(TypedDict):
+    contract: Contract
+    functions: List[Function]
+    variables: List[Variable]
+
+
 # pylint: disable=too-many-locals
 def compare(
     v1: Contract, v2: Contract
 ) -> Tuple[
-    List[Variable], List[Variable], List[Variable], List[Function], List[Function], List[Function]
+    List[Variable],
+    List[Variable],
+    List[Variable],
+    List[Function],
+    List[Function],
+    List[Function],
+    List[TaintedExternalContract],
 ]:
     """
     Compares two versions of a contract. Most useful for upgradeable (logic) contracts,
@@ -159,16 +174,137 @@ def compare(
         ):
             tainted_variables.append(var)
 
+    # Find all external contracts and functions called by new/modified/tainted functions
+    tainted_contracts = tainted_external_contracts(
+        new_functions + modified_functions + tainted_functions
+    )
+
     return (
         missing_vars_in_v2,
         new_variables,
         tainted_variables,
         new_functions,
         modified_functions,
         tainted_functions,
+        tainted_contracts,
     )
 
 
+def tainted_external_contracts(funcs: List[Function]) -> List[TaintedExternalContract]:
+    """
+    Takes a list of functions from one contract, finds any calls in these to functions in external contracts,
+    and determines which variables and functions in the external contracts are tainted by these external calls.
+    Args:
+        funcs: a list of Function objects to search for external calls.
+
+    Returns:
+        TaintedExternalContract(TypedDict) (
+            contract: Contract,
+            functions: List[Function],
+            variables: List[Variable]
+        )
+    """
+    tainted_contracts: dict[str, TaintedExternalContract] = {}
+    tainted_list: list[TaintedExternalContract] = []
+
+    for func in funcs:
+        for contract, target in func.all_high_level_calls():
+            if contract.is_library:
+                continue
+            if contract.name not in tainted_contracts:
+                tainted_contracts[contract.name] = TaintedExternalContract(
+                    contract=contract, functions=[], variables=[]
+                )
+            if (
+                isinstance(target, Function)
+                and target not in funcs
+                and target not in tainted_contracts[contract.name]["functions"]
+                and not (target.is_constructor or target.is_fallback or target.is_receive)
+            ):
+                tainted_contracts[contract.name]["functions"].append(target)
+                for var in target.all_state_variables_written():
+                    if var not in tainted_contracts[contract.name]["variables"]:
+                        tainted_contracts[contract.name]["variables"].append(var)
+            elif (
+                isinstance(target, Variable)
+                and target not in tainted_contracts[contract.name]["variables"]
+                and not (target.is_constant or target.is_immutable)
+            ):
+                tainted_contracts[contract.name]["variables"].append(target)
+    for c in tainted_contracts.items():
+        if len(c[1]["functions"]) == 0 and len(c[1]["variables"]) == 0:
+            continue
+        tainted_list.append(c[1])
+        contract = c[1]["contract"]
+        variables = c[1]["variables"]
+        for var in variables:
+            read_write = set(
+                contract.get_functions_reading_from_variable(var)
+                + contract.get_functions_writing_to_variable(var)
+            )
+            for f in read_write:
+                if f not in tainted_contracts[contract.name]["functions"] and not (
+                    f.is_constructor or f.is_fallback or f.is_receive
+                ):
+                    tainted_contracts[contract.name]["functions"].append(f)
+    return tainted_list
+
+
+def tainted_inheriting_contracts(
+    tainted_contracts: List[TaintedExternalContract], contracts: List[Contract] = None
+) -> List[TaintedExternalContract]:
+    """
+    Takes a list of TaintedExternalContract obtained from tainted_external_contracts, and finds any contracts which
+    inherit a tainted contract, as well as any functions that call tainted functions or read tainted variables in
+    the inherited contract.
+    Args:
+        tainted_contracts: the list obtained from `tainted_external_contracts` or `compare`.
+        contracts: (optional) the list of contracts to check for inheritance. If not provided, defaults to
+                    `contract.compilation_unit.contracts` for each contract in tainted_contracts.
+
+    Returns:
+        An updated list of TaintedExternalContract, including all from the input list.
+    """
+    for tainted in tainted_contracts:
+        contract = tainted["contract"]
+        if contracts is None:
+            contracts = contract.compilation_unit.contracts
+        contracts = [
+            c
+            for c in contracts
+            if c.name not in tainted_contracts and c.name in [i.name for i in c.inheritance]
+        ]
+        for c in contracts:
+            new_taint = TaintedExternalContract(contract=c, functions=[], variables=[])
+            for f in c.functions_declared:
+                internal_calls = f.all_internal_calls()
+                if any(
+                    str(call) == str(t) for t in tainted["functions"] for call in internal_calls
+                ) or any(
+                    str(var) == str(t)
+                    for t in tainted["variables"]
+                    for var in f.all_state_variables_read() + f.all_state_variables_written()
+                ):
+                    new_taint["functions"].append(f)
+            for f in new_taint["functions"]:
+                for var in f.all_state_variables_read() + f.all_state_variables_written():
+                    if not (var in tainted["variables"] or var in new_taint["variables"]):
+                        new_taint["variables"].append(var)
+            for var in new_taint["variables"]:
+                read_write = set(
+                    contract.get_functions_reading_from_variable(var)
+                    + contract.get_functions_writing_to_variable(var)
+                )
+                for f in read_write:
+                    if f not in tainted["functions"] + new_taint["functions"] and not (
+                        f.is_constructor or f.is_fallback or f.is_receive
+                    ):
+                        new_taint["functions"].append(f)
+            if len(new_taint["functions"]) > 0:
+                tainted_contracts.append(new_taint)
+    return tainted_contracts
+
+
 def get_missing_vars(v1: Contract, v2: Contract) -> List[StateVariable]:
     """
     Gets all non-constant/immutable StateVariables that appear in v1 but not v2
@@ -273,7 +409,7 @@ def encode_ir_for_compare(ir: Operation) -> str:
     if isinstance(ir, Assignment):
         return f"({encode_var_for_compare(ir.lvalue)}):=({encode_var_for_compare(ir.rvalue)})"
     if isinstance(ir, Index):
-        return f"index({ntype(ir.index_type)})"
+        return f"index({ntype(ir.variable_right.type)})"
     if isinstance(ir, Member):
         return "member"  # .format(ntype(ir._type))
     if isinstance(ir, Length):
@@ -398,6 +534,7 @@ def get_proxy_implementation_var(proxy: Contract) -> Optional[Variable]:
         try:
             delegate = next(var for var in dependencies if isinstance(var, StateVariable))
         except StopIteration:
+            # TODO: Handle cases where get_dependencies doesn't return any state variables.
             return delegate
     return delegate
 

tests/unit/utils/test_upgradeability_util.py

@@ -22,8 +22,16 @@ def test_upgrades_compare() -> None:
     sl = Slither(os.path.join(TEST_DATA_DIR, "TestUpgrades-0.8.2.sol"))
     v1 = sl.get_contract_from_name("ContractV1")[0]
     v2 = sl.get_contract_from_name("ContractV2")[0]
-    missing_vars, new_vars, tainted_vars, new_funcs, modified_funcs, tainted_funcs = compare(v1, v2)
-    assert len(missing_vars) == 0
+    (
+        missing_vars,
+        new_vars,
+        tainted_vars,
+        new_funcs,
+        modified_funcs,
+        tainted_funcs,
+        tainted_contracts,
+    ) = compare(v1, v2)
+    assert len(missing_vars) == len(tainted_contracts) == 0
     assert new_vars == [v2.get_state_variable_from_name("stateC")]
     assert tainted_vars == [
         v2.get_state_variable_from_name("stateB"),