Merge pull request #1816 from webthethird/dev-upgradeability-util-cross-contract-taint

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

slither/utils/upgradeability.py

@@ -19,10 +19,12 @@ from slither.core.variables.local_variable_init_from_tuple import LocalVariableI
 from slither.core.variables.state_variable import StateVariable
 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 +63,42 @@ from slither.slithir.variables import (
 from slither.tools.read_storage.read_storage import SlotInfo, SlitherReadStorage
 
 
+class TaintedExternalContract:
+    def __init__(self, contract: "Contract") -> None:
+        self._contract: Contract = contract
+        self._tainted_functions: List[Function] = []
+        self._tainted_variables: List[Variable] = []
+
+    @property
+    def contract(self) -> Contract:
+        return self._contract
+
+    @property
+    def tainted_functions(self) -> List[Function]:
+        return self._tainted_functions
+
+    def add_tainted_function(self, f: Function):
+        self._tainted_functions.append(f)
+
+    @property
+    def tainted_variables(self) -> List[Variable]:
+        return self._tainted_variables
+
+    def add_tainted_variable(self, v: Variable):
+        self._tainted_variables.append(v)
+
+
 # pylint: disable=too-many-locals
 def compare(
-    v1: Contract, v2: Contract
+    v1: Contract, v2: Contract, include_external: bool = False
 ) -> 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,
@@ -74,6 +107,7 @@ def compare(
     Args:
         v1: Original version of (upgradeable) contract
         v2: Updated version of (upgradeable) contract
+        include_external: Optional flag to enable cross-contract external taint analysis
 
     Returns:
         missing-vars-in-v2: list[Variable],
@@ -82,6 +116,7 @@ def compare(
         new-functions: list[Function],
         modified-functions: list[Function],
         tainted-functions: list[Function]
+        tainted-contracts: list[TaintedExternalContract]
     """
 
     order_vars1 = [
@@ -113,17 +148,13 @@ def compare(
         if sig not in func_sigs1:
             new_modified_functions.append(function)
             new_functions.append(function)
-            new_modified_function_vars += (
-                function.state_variables_read + function.state_variables_written
-            )
+            new_modified_function_vars += function.all_state_variables_written()
         elif not function.is_constructor_variables and is_function_modified(
             orig_function, function
         ):
             new_modified_functions.append(function)
             modified_functions.append(function)
-            new_modified_function_vars += (
-                function.state_variables_read + function.state_variables_written
-            )
+            new_modified_function_vars += function.all_state_variables_written()
 
     # Find all unmodified functions that call a modified function or read/write the
     # same state variable(s) as a new/modified function, i.e., tainted functions
@@ -140,25 +171,28 @@ def compare(
         tainted_vars = [
             var
             for var in set(new_modified_function_vars)
-            if var in function.variables_read_or_written
+            if var in function.all_state_variables_read() + function.all_state_variables_written()
             and not var.is_constant
             and not var.is_immutable
         ]
         if len(modified_calls) > 0 or len(tainted_vars) > 0:
             tainted_functions.append(function)
 
-    # Find all new or tainted variables, i.e., variables that are read or written by a new/modified/tainted function
+    # Find all new or tainted variables, i.e., variables that are written by a new/modified/tainted function
     for var in order_vars2:
-        read_by = v2.get_functions_reading_from_variable(var)
         written_by = v2.get_functions_writing_to_variable(var)
-        if v1.get_state_variable_from_name(var.name) is None:
+        if next((v for v in v1.state_variables_ordered if v.name == var.name), None) is None:
             new_variables.append(var)
-        elif any(
-            func in read_by or func in written_by
-            for func in new_modified_functions + tainted_functions
-        ):
+        elif any(func in written_by for func in new_modified_functions + tainted_functions):
             tainted_variables.append(var)
 
+    tainted_contracts = []
+    if include_external:
+        # 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,
@@ -166,9 +200,137 @@ def compare(
         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() (
+            contract: Contract,
+            tainted_functions: List[TaintedFunction],
+            tainted_variables: List[TaintedVariable]
+        )
+    """
+    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:
+                # Not interested in library calls
+                continue
+            if contract.name not in tainted_contracts:
+                # A contract may be tainted by multiple function calls - only make one TaintedExternalContract object
+                tainted_contracts[contract.name] = TaintedExternalContract(contract)
+            if (
+                isinstance(target, Function)
+                and target not in funcs
+                and target not in (f for f in tainted_contracts[contract.name].tainted_functions)
+                and not (target.is_constructor or target.is_fallback or target.is_receive)
+            ):
+                # Found a high-level call to a new tainted function
+                tainted_contracts[contract.name].add_tainted_function(target)
+                for var in target.all_state_variables_written():
+                    # Consider as tainted all variables written by the tainted function
+                    if var not in (v for v in tainted_contracts[contract.name].tainted_variables):
+                        tainted_contracts[contract.name].add_tainted_variable(var)
+            elif (
+                isinstance(target, StateVariable)
+                and target not in (v for v in tainted_contracts[contract.name].tainted_variables)
+                and not (target.is_constant or target.is_immutable)
+            ):
+                # Found a new high-level call to a public state variable getter
+                tainted_contracts[contract.name].add_tainted_variable(target)
+    for c in tainted_contracts.values():
+        tainted_list.append(c)
+        contract = c.contract
+        variables = c.tainted_variables
+        for var in variables:
+            # For each tainted variable, consider as tainted any function that reads or writes to it
+            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].tainted_functions and not (
+                    f.is_constructor or f.is_fallback or f.is_receive
+                ):
+                    c.add_tainted_function(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
+        check_contracts = contracts
+        if contracts is None:
+            check_contracts = contract.compilation_unit.contracts
+        # We are only interested in checking contracts that inherit a tainted contract
+        check_contracts = [
+            c
+            for c in check_contracts
+            if c.name not in [t.contract.name for t in tainted_contracts]
+            and contract.name in [i.name for i in c.inheritance]
+        ]
+        for c in check_contracts:
+            new_taint = TaintedExternalContract(c)
+            for f in c.functions_declared:
+                # Search for functions that call an inherited tainted function or access an inherited tainted variable
+                internal_calls = [c for c in f.all_internal_calls() if isinstance(c, Function)]
+                if any(
+                    call.canonical_name == t.canonical_name
+                    for t in tainted.tainted_functions
+                    for call in internal_calls
+                ) or any(
+                    var.canonical_name == t.canonical_name
+                    for t in tainted.tainted_variables
+                    for var in f.all_state_variables_read() + f.all_state_variables_written()
+                ):
+                    new_taint.add_tainted_function(f)
+            for f in new_taint.tainted_functions:
+                # For each newly found tainted function, consider as tainted any variable it writes to
+                for var in f.all_state_variables_written():
+                    if var not in (
+                        v for v in tainted.tainted_variables + new_taint.tainted_variables
+                    ):
+                        new_taint.add_tainted_variable(var)
+            for var in new_taint.tainted_variables:
+                # For each newly found tainted variable, consider as tainted any function that reads or writes to it
+                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 (
+                        t for t in tainted.tainted_functions + new_taint.tainted_functions
+                    ) and not (f.is_constructor or f.is_fallback or f.is_receive):
+                        new_taint.add_tainted_function(f)
+            if len(new_taint.tainted_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
@@ -407,6 +569,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_data/upgradeability_util/src/ContractV2.sol

@@ -1,6 +1,7 @@
 pragma solidity ^0.8.2;
 
 import "./ProxyStorage.sol";
+import "./ERC20.sol";
 
 contract ContractV2 is ProxyStorage {
     uint private stateA = 0;
@@ -38,4 +39,8 @@ contract ContractV2 is ProxyStorage {
     function checkB() internal returns (bool) {
         return stateB == 32;
     }
+
+    function erc20Transfer(address erc20, address to, uint256 amount) public returns (bool) {
+        return ERC20(erc20).transfer(to, amount);
+    }
 }

tests/unit/utils/test_data/upgradeability_util/src/ERC20.sol

@@ -0,0 +1,376 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.8.0) (token/ERC20/ERC20.sol)
+
+pragma solidity ^0.8.0;
+
+/**
+ * @dev Implementation of the {IERC20} interface.
+ *
+ * This implementation is agnostic to the way tokens are created. This means
+ * that a supply mechanism has to be added in a derived contract using {_mint}.
+ * For a generic mechanism see {ERC20PresetMinterPauser}.
+ *
+ * TIP: For a detailed writeup see our guide
+ * https://forum.openzeppelin.com/t/how-to-implement-erc20-supply-mechanisms/226[How
+ * to implement supply mechanisms].
+ *
+ * We have followed general OpenZeppelin Contracts guidelines: functions revert
+ * instead returning `false` on failure. This behavior is nonetheless
+ * conventional and does not conflict with the expectations of ERC20
+ * applications.
+ *
+ * Additionally, an {Approval} event is emitted on calls to {transferFrom}.
+ * This allows applications to reconstruct the allowance for all accounts just
+ * by listening to said events. Other implementations of the EIP may not emit
+ * these events, as it isn't required by the specification.
+ *
+ * Finally, the non-standard {decreaseAllowance} and {increaseAllowance}
+ * functions have been added to mitigate the well-known issues around setting
+ * allowances. See {IERC20-approve}.
+ */
+contract ERC20 {
+    mapping(address => uint256) private _balances;
+
+    mapping(address => mapping(address => uint256)) private _allowances;
+
+    uint256 private _totalSupply;
+
+    string private _name;
+    string private _symbol;
+
+    /**
+     * @dev Sets the values for {name} and {symbol}.
+     *
+     * The default value of {decimals} is 18. To select a different value for
+     * {decimals} you should overload it.
+     *
+     * All two of these values are immutable: they can only be set once during
+     * construction.
+     */
+    constructor(string memory name_, string memory symbol_) {
+        _name = name_;
+        _symbol = symbol_;
+    }
+
+    /**
+     * @dev Returns the name of the token.
+     */
+    function name() public view virtual returns (string memory) {
+        return _name;
+    }
+
+    /**
+     * @dev Returns the symbol of the token, usually a shorter version of the
+     * name.
+     */
+    function symbol() public view virtual returns (string memory) {
+        return _symbol;
+    }
+
+    /**
+     * @dev Returns the number of decimals used to get its user representation.
+     * For example, if `decimals` equals `2`, a balance of `505` tokens should
+     * be displayed to a user as `5.05` (`505 / 10 ** 2`).
+     *
+     * Tokens usually opt for a value of 18, imitating the relationship between
+     * Ether and Wei. This is the value {ERC20} uses, unless this function is
+     * overridden;
+     *
+     * NOTE: This information is only used for _display_ purposes: it in
+     * no way affects any of the arithmetic of the contract, including
+     * {IERC20-balanceOf} and {IERC20-transfer}.
+     */
+    function decimals() public view virtual returns (uint8) {
+        return 18;
+    }
+
+    /**
+     * @dev See {IERC20-totalSupply}.
+     */
+    function totalSupply() public view virtual returns (uint256) {
+        return _totalSupply;
+    }
+
+    /**
+     * @dev See {IERC20-balanceOf}.
+     */
+    function balanceOf(address account) public view virtual returns (uint256) {
+        return _balances[account];
+    }
+
+    /**
+     * @dev See {IERC20-transfer}.
+     *
+     * Requirements:
+     *
+     * - `to` cannot be the zero address.
+     * - the caller must have a balance of at least `amount`.
+     */
+    function transfer(address to, uint256 amount) public virtual returns (bool) {
+        address owner = msg.sender;
+        _transfer(owner, to, amount);
+        return true;
+    }
+
+    /**
+     * @dev See {IERC20-allowance}.
+     */
+    function allowance(address owner, address spender) public view virtual returns (uint256) {
+        return _allowances[owner][spender];
+    }
+
+    /**
+     * @dev See {IERC20-approve}.
+     *
+     * NOTE: If `amount` is the maximum `uint256`, the allowance is not updated on
+     * `transferFrom`. This is semantically equivalent to an infinite approval.
+     *
+     * Requirements:
+     *
+     * - `spender` cannot be the zero address.
+     */
+    function approve(address spender, uint256 amount) public virtual returns (bool) {
+        address owner = msg.sender;
+        _approve(owner, spender, amount);
+        return true;
+    }
+
+    /**
+     * @dev See {IERC20-transferFrom}.
+     *
+     * Emits an {Approval} event indicating the updated allowance. This is not
+     * required by the EIP. See the note at the beginning of {ERC20}.
+     *
+     * NOTE: Does not update the allowance if the current allowance
+     * is the maximum `uint256`.
+     *
+     * Requirements:
+     *
+     * - `from` and `to` cannot be the zero address.
+     * - `from` must have a balance of at least `amount`.
+     * - the caller must have allowance for ``from``'s tokens of at least
+     * `amount`.
+     */
+    function transferFrom(
+        address from,
+        address to,
+        uint256 amount
+    ) public virtual returns (bool) {
+        address spender = msg.sender;
+        _spendAllowance(from, spender, amount);
+        _transfer(from, to, amount);
+        return true;
+    }
+
+    /**
+     * @dev Atomically increases the allowance granted to `spender` by the caller.
+     *
+     * This is an alternative to {approve} that can be used as a mitigation for
+     * problems described in {IERC20-approve}.
+     *
+     * Emits an {Approval} event indicating the updated allowance.
+     *
+     * Requirements:
+     *
+     * - `spender` cannot be the zero address.
+     */
+    function increaseAllowance(address spender, uint256 addedValue) public virtual returns (bool) {
+        address owner = msg.sender;
+        _approve(owner, spender, allowance(owner, spender) + addedValue);
+        return true;
+    }
+
+    /**
+     * @dev Atomically decreases the allowance granted to `spender` by the caller.
+     *
+     * This is an alternative to {approve} that can be used as a mitigation for
+     * problems described in {IERC20-approve}.
+     *
+     * Emits an {Approval} event indicating the updated allowance.
+     *
+     * Requirements:
+     *
+     * - `spender` cannot be the zero address.
+     * - `spender` must have allowance for the caller of at least
+     * `subtractedValue`.
+     */
+    function decreaseAllowance(address spender, uint256 subtractedValue) public virtual returns (bool) {
+        address owner = msg.sender;
+        uint256 currentAllowance = allowance(owner, spender);
+        require(currentAllowance >= subtractedValue, "ERC20: decreased allowance below zero");
+        unchecked {
+            _approve(owner, spender, currentAllowance - subtractedValue);
+        }
+
+        return true;
+    }
+
+    /**
+     * @dev Moves `amount` of tokens from `from` to `to`.
+     *
+     * This internal function is equivalent to {transfer}, and can be used to
+     * e.g. implement automatic token fees, slashing mechanisms, etc.
+     *
+     * Emits a {Transfer} event.
+     *
+     * Requirements:
+     *
+     * - `from` cannot be the zero address.
+     * - `to` cannot be the zero address.
+     * - `from` must have a balance of at least `amount`.
+     */
+    function _transfer(
+        address from,
+        address to,
+        uint256 amount
+    ) internal virtual {
+        require(from != address(0), "ERC20: transfer from the zero address");
+        require(to != address(0), "ERC20: transfer to the zero address");
+
+        _beforeTokenTransfer(from, to, amount);
+
+        uint256 fromBalance = _balances[from];
+        require(fromBalance >= amount, "ERC20: transfer amount exceeds balance");
+        unchecked {
+            _balances[from] = fromBalance - amount;
+            // Overflow not possible: the sum of all balances is capped by totalSupply, and the sum is preserved by
+            // decrementing then incrementing.
+            _balances[to] += amount;
+        }
+        _afterTokenTransfer(from, to, amount);
+    }
+
+    /** @dev Creates `amount` tokens and assigns them to `account`, increasing
+     * the total supply.
+     *
+     * Emits a {Transfer} event with `from` set to the zero address.
+     *
+     * Requirements:
+     *
+     * - `account` cannot be the zero address.
+     */
+    function _mint(address account, uint256 amount) internal virtual {
+        require(account != address(0), "ERC20: mint to the zero address");
+
+        _beforeTokenTransfer(address(0), account, amount);
+
+        _totalSupply += amount;
+        unchecked {
+            // Overflow not possible: balance + amount is at most totalSupply + amount, which is checked above.
+            _balances[account] += amount;
+        }
+        _afterTokenTransfer(address(0), account, amount);
+    }
+
+    /**
+     * @dev Destroys `amount` tokens from `account`, reducing the
+     * total supply.
+     *
+     * Emits a {Transfer} event with `to` set to the zero address.
+     *
+     * Requirements:
+     *
+     * - `account` cannot be the zero address.
+     * - `account` must have at least `amount` tokens.
+     */
+    function _burn(address account, uint256 amount) internal virtual {
+        require(account != address(0), "ERC20: burn from the zero address");
+
+        _beforeTokenTransfer(account, address(0), amount);
+
+        uint256 accountBalance = _balances[account];
+        require(accountBalance >= amount, "ERC20: burn amount exceeds balance");
+        unchecked {
+            _balances[account] = accountBalance - amount;
+            // Overflow not possible: amount <= accountBalance <= totalSupply.
+            _totalSupply -= amount;
+        }
+        _afterTokenTransfer(account, address(0), amount);
+    }
+
+    /**
+     * @dev Sets `amount` as the allowance of `spender` over the `owner` s tokens.
+     *
+     * This internal function is equivalent to `approve`, and can be used to
+     * e.g. set automatic allowances for certain subsystems, etc.
+     *
+     * Emits an {Approval} event.
+     *
+     * Requirements:
+     *
+     * - `owner` cannot be the zero address.
+     * - `spender` cannot be the zero address.
+     */
+    function _approve(
+        address owner,
+        address spender,
+        uint256 amount
+    ) internal virtual {
+        require(owner != address(0), "ERC20: approve from the zero address");
+        require(spender != address(0), "ERC20: approve to the zero address");
+
+        _allowances[owner][spender] = amount;
+    }
+
+    /**
+     * @dev Updates `owner` s allowance for `spender` based on spent `amount`.
+     *
+     * Does not update the allowance amount in case of infinite allowance.
+     * Revert if not enough allowance is available.
+     *
+     * Might emit an {Approval} event.
+     */
+    function _spendAllowance(
+        address owner,
+        address spender,
+        uint256 amount
+    ) internal virtual {
+        uint256 currentAllowance = allowance(owner, spender);
+        if (currentAllowance != type(uint256).max) {
+            require(currentAllowance >= amount, "ERC20: insufficient allowance");
+            unchecked {
+                _approve(owner, spender, currentAllowance - amount);
+            }
+        }
+    }
+
+    /**
+     * @dev Hook that is called before any transfer of tokens. This includes
+     * minting and burning.
+     *
+     * Calling conditions:
+     *
+     * - when `from` and `to` are both non-zero, `amount` of ``from``'s tokens
+     * will be transferred to `to`.
+     * - when `from` is zero, `amount` tokens will be minted for `to`.
+     * - when `to` is zero, `amount` of ``from``'s tokens will be burned.
+     * - `from` and `to` are never both zero.
+     *
+     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
+     */
+    function _beforeTokenTransfer(
+        address from,
+        address to,
+        uint256 amount
+    ) internal virtual {}
+
+    /**
+     * @dev Hook that is called after any transfer of tokens. This includes
+     * minting and burning.
+     *
+     * Calling conditions:
+     *
+     * - when `from` and `to` are both non-zero, `amount` of ``from``'s tokens
+     * has been transferred to `to`.
+     * - when `from` is zero, `amount` tokens have been minted for `to`.
+     * - when `to` is zero, `amount` of ``from``'s tokens have been burned.
+     * - `from` and `to` are never both zero.
+     *
+     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
+     */
+    function _afterTokenTransfer(
+        address from,
+        address to,
+        uint256 amount
+    ) internal virtual {}
+}

tests/unit/utils/test_upgradeability_util.py

@@ -20,19 +20,41 @@ def test_upgrades_compare(solc_binary_path) -> None:
     sl = Slither(os.path.join(TEST_DATA_DIR, "TestUpgrades-0.8.2.sol"), solc=solc_path)
     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)
+    (
+        missing_vars,
+        new_vars,
+        tainted_vars,
+        new_funcs,
+        modified_funcs,
+        tainted_funcs,
+        tainted_contracts,
+    ) = compare(v1, v2, include_external=True)
     assert len(missing_vars) == 0
     assert new_vars == [v2.get_state_variable_from_name("stateC")]
     assert tainted_vars == [
-        v2.get_state_variable_from_name("stateB"),
         v2.get_state_variable_from_name("bug"),
     ]
-    assert new_funcs == [v2.get_function_from_signature("i()")]
+    assert new_funcs == [
+        v2.get_function_from_signature("i()"),
+        v2.get_function_from_signature("erc20Transfer(address,address,uint256)"),
+    ]
     assert modified_funcs == [v2.get_function_from_signature("checkB()")]
     assert tainted_funcs == [
-        v2.get_function_from_signature("g(uint256)"),
         v2.get_function_from_signature("h()"),
     ]
+    erc20 = sl.get_contract_from_name("ERC20")[0]
+    assert len(tainted_contracts) == 1
+    assert tainted_contracts[0].contract == erc20
+    assert set(tainted_contracts[0].tainted_functions) == {
+        erc20.get_function_from_signature("transfer(address,uint256)"),
+        erc20.get_function_from_signature("_transfer(address,address,uint256)"),
+        erc20.get_function_from_signature("_burn(address,uint256)"),
+        erc20.get_function_from_signature("balanceOf(address)"),
+        erc20.get_function_from_signature("_mint(address,uint256)"),
+    }
+    assert tainted_contracts[0].tainted_variables == [
+        erc20.get_state_variable_from_name("_balances")
+    ]
 
 
 def test_upgrades_implementation_var(solc_binary_path) -> None: