Merge branch 'dev' into dev-super-linter

.github/workflows/docker.yml

@@ -30,7 +30,7 @@ jobs:
 
       - name: Set Docker metadata
         id: metadata
-        uses: docker/metadata-action@v4
+        uses: docker/metadata-action@v5
         with:
           images: |
             ghcr.io/${{ github.repository }}

slither/__main__.py

@@ -268,9 +268,10 @@ def choose_printers(
 ###################################################################################
 
 
-def parse_filter_paths(args: argparse.Namespace) -> List[str]:
-    if args.filter_paths:
-        return args.filter_paths.split(",")
+def parse_filter_paths(args: argparse.Namespace, filter_path: bool) -> List[str]:
+    paths = args.filter_paths if filter_path else args.include_paths
+    if paths:
+        return paths.split(",")
     return []
 
 
@@ -307,6 +308,7 @@ def parse_args(
         "Checklist (consider using https://github.com/crytic/slither-action)"
     )
     group_misc = parser.add_argument_group("Additional options")
+    group_filters = parser.add_mutually_exclusive_group()
 
     group_detector.add_argument(
         "--detect",
@@ -518,14 +520,6 @@ def parse_args(
         default=defaults_flag_in_config["disable_color"],
     )
 
-    group_misc.add_argument(
-        "--filter-paths",
-        help="Regex filter to exclude detector results matching file path e.g. (mocks/|test/)",
-        action="store",
-        dest="filter_paths",
-        default=defaults_flag_in_config["filter_paths"],
-    )
-
     group_misc.add_argument(
         "--triage-mode",
         help="Run triage mode (save results in triage database)",
@@ -579,6 +573,22 @@ def parse_args(
         default=defaults_flag_in_config["no_fail"],
     )
 
+    group_filters.add_argument(
+        "--filter-paths",
+        help="Regex filter to exclude detector results matching file path e.g. (mocks/|test/)",
+        action="store",
+        dest="filter_paths",
+        default=defaults_flag_in_config["filter_paths"],
+    )
+
+    group_filters.add_argument(
+        "--include-paths",
+        help="Regex filter to include detector results matching file path e.g. (src/|contracts/). Opposite of --filter-paths",
+        action="store",
+        dest="include_paths",
+        default=defaults_flag_in_config["include_paths"],
+    )
+
     codex.init_parser(parser)
 
     # debugger command
@@ -631,7 +641,8 @@ def parse_args(
     args = parser.parse_args()
     read_config_file(args)
 
-    args.filter_paths = parse_filter_paths(args)
+    args.filter_paths = parse_filter_paths(args, True)
+    args.include_paths = parse_filter_paths(args, False)
 
     # Verify our json-type output is valid
     args.json_types = set(args.json_types.split(","))  # type:ignore

slither/core/cfg/node.py

@@ -11,6 +11,7 @@
     SolidityFunction,
 )
 from slither.core.expressions.expression import Expression
+from slither.core.expressions import CallExpression, Identifier, AssignmentOperation
 from slither.core.solidity_types import ElementaryType
 from slither.core.source_mapping.source_mapping import SourceMapping
 from slither.core.variables.local_variable import LocalVariable
@@ -898,6 +899,21 @@ def _find_read_write_call(self) -> None:  # pylint: disable=too-many-statements
                 # TODO: consider removing dependancy of solidity_call to internal_call
                 self._solidity_calls.append(ir.function)
                 self._internal_calls.append(ir.function)
+            if (
+                isinstance(ir, SolidityCall)
+                and ir.function == SolidityFunction("sstore(uint256,uint256)")
+                and isinstance(ir.node.expression, CallExpression)
+                and isinstance(ir.node.expression.arguments[0], Identifier)
+            ):
+                self._vars_written.append(ir.arguments[0])
+            if (
+                isinstance(ir, SolidityCall)
+                and ir.function == SolidityFunction("sload(uint256)")
+                and isinstance(ir.node.expression, AssignmentOperation)
+                and isinstance(ir.node.expression.expression_right, CallExpression)
+                and isinstance(ir.node.expression.expression_right.arguments[0], Identifier)
+            ):
+                self._vars_read.append(ir.arguments[0])
             if isinstance(ir, LowLevelCall):
                 assert isinstance(ir.destination, (Variable, SolidityVariable))
                 self._low_level_calls.append((ir.destination, str(ir.function_name.value)))

slither/core/slither_core.py

@@ -62,6 +62,7 @@ def __init__(self) -> None:
         # Multiple time the same result, so we remove duplicates
         self._currently_seen_resuts: Set[str] = set()
         self._paths_to_filter: Set[str] = set()
+        self._paths_to_include: Set[str] = set()
 
         self._crytic_compile: Optional[CryticCompile] = None
 
@@ -411,25 +412,29 @@ def valid_result(self, r: Dict) -> bool:
             if "source_mapping" in elem
         ]
 
-        # Use POSIX-style paths so that filter_paths works across different
+        # Use POSIX-style paths so that filter_paths|include_paths works across different
         # OSes. Convert to a list so elements don't get consumed and are lost
         # while evaluating the first pattern
         source_mapping_elements = list(
             map(lambda x: pathlib.Path(x).resolve().as_posix() if x else x, source_mapping_elements)
         )
-        matching = False
+        (matching, paths, msg_err) = (
+            (True, self._paths_to_include, "--include-paths")
+            if self._paths_to_include
+            else (False, self._paths_to_filter, "--filter-paths")
+        )
 
-        for path in self._paths_to_filter:
+        for path in paths:
             try:
                 if any(
                     bool(re.search(_relative_path_format(path), src_mapping))
                     for src_mapping in source_mapping_elements
                 ):
-                    matching = True
+                    matching = not matching
                     break
             except re.error:
                 logger.error(
-                    f"Incorrect regular expression for --filter-paths {path}."
+                    f"Incorrect regular expression for {msg_err} {path}."
                     "\nSlither supports the Python re format"
                     ": https://docs.python.org/3/library/re.html"
                 )
@@ -500,6 +505,13 @@ def add_path_to_filter(self, path: str):
         """
         self._paths_to_filter.add(path)
 
+    def add_path_to_include(self, path: str):
+        """
+        Add path to include
+        Path are used through direct comparison (no regex)
+        """
+        self._paths_to_include.add(path)
+
     # endregion
     ###################################################################################
     ###################################################################################

slither/detectors/all_detectors.py

@@ -97,3 +97,4 @@
 from .operations.incorrect_exp import IncorrectOperatorExponentiation
 from .statements.tautological_compare import TautologicalCompare
 from .statements.return_bomb import ReturnBomb
+from .functions.out_of_order_retryable import OutOfOrderRetryable

slither/detectors/functions/out_of_order_retryable.py

@@ -0,0 +1,155 @@
+from typing import List
+
+from slither.core.cfg.node import Node
+from slither.core.declarations import Function, FunctionContract
+from slither.slithir.operations import HighLevelCall
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.utils.output import Output
+
+
+class OutOfOrderRetryable(AbstractDetector):
+
+    ARGUMENT = "out-of-order-retryable"
+    HELP = "Out-of-order retryable transactions"
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#out-of-order-retryable-transactions"
+
+    WIKI_TITLE = "Out-of-order retryable transactions"
+    WIKI_DESCRIPTION = "Out-of-order retryable transactions"
+
+    # region wiki_exploit_scenario
+    WIKI_EXPLOIT_SCENARIO = """
+```solidity
+contract L1 {
+    function doStuffOnL2() external {
+        // Retryable A
+        IInbox(inbox).createRetryableTicket({
+            to: l2contract,
+            l2CallValue: 0,
+            maxSubmissionCost: maxSubmissionCost,
+            excessFeeRefundAddress: msg.sender,
+            callValueRefundAddress: msg.sender,
+            gasLimit: gasLimit,
+            maxFeePerGas: maxFeePerGas,
+            data: abi.encodeCall(l2contract.claim_rewards, ())
+        });
+        // Retryable B
+        IInbox(inbox).createRetryableTicket({
+            to: l2contract,
+            l2CallValue: 0,
+            maxSubmissionCost: maxSubmissionCost,
+            excessFeeRefundAddress: msg.sender,
+            callValueRefundAddress: msg.sender,
+            gasLimit: gas,
+            maxFeePerGas: maxFeePerGas,
+            data: abi.encodeCall(l2contract.unstake, ())
+        });
+    }
+}
+
+contract L2 {
+    function claim_rewards() public {
+        // rewards is computed based on balance and staking period
+        uint unclaimed_rewards = _compute_and_update_rewards();
+        token.safeTransfer(msg.sender, unclaimed_rewards);
+    }
+
+    // Call claim_rewards before unstaking, otherwise you lose your rewards
+    function unstake() public {
+        _free_rewards(); // clean up rewards related variables
+        balance = balance[msg.sender];
+        balance[msg.sender] = 0;
+        staked_token.safeTransfer(msg.sender, balance);
+    }
+}
+```
+Bob calls `doStuffOnL2` but the first retryable ticket calling `claim_rewards` fails. The second retryable ticket calling `unstake` is executed successfully. As a result, Bob loses his rewards."""
+    # endregion wiki_exploit_scenario
+
+    WIKI_RECOMMENDATION = "Do not rely on the order or successful execution of retryable tickets."
+
+    key = "OUTOFORDERRETRYABLE"
+
+    # pylint: disable=too-many-branches
+    def _detect_multiple_tickets(
+        self, function: FunctionContract, node: Node, visited: List[Node]
+    ) -> None:
+        if node in visited:
+            return
+
+        visited = visited + [node]
+
+        fathers_context = []
+
+        for father in node.fathers:
+            if self.key in father.context:
+                fathers_context += father.context[self.key]
+
+        # Exclude path that dont bring further information
+        if node in self.visited_all_paths:
+            if all(f_c in self.visited_all_paths[node] for f_c in fathers_context):
+                return
+        else:
+            self.visited_all_paths[node] = []
+
+        self.visited_all_paths[node] = self.visited_all_paths[node] + fathers_context
+
+        if self.key not in node.context:
+            node.context[self.key] = fathers_context
+
+        # include ops from internal function calls
+        internal_ops = []
+        for internal_call in node.internal_calls:
+            if isinstance(internal_call, Function):
+                internal_ops += internal_call.all_slithir_operations()
+
+        # analyze node for retryable tickets
+        for ir in node.irs + internal_ops:
+            if (
+                isinstance(ir, HighLevelCall)
+                and isinstance(ir.function, Function)
+                and ir.function.name
+                in [
+                    "createRetryableTicket",
+                    "outboundTransferCustomRefund",
+                    "unsafeCreateRetryableTicket",
+                ]
+            ):
+                node.context[self.key].append(node)
+
+        if len(node.context[self.key]) > 1:
+            self.results.append(node.context[self.key])
+            return
+
+        for son in node.sons:
+            self._detect_multiple_tickets(function, son, visited)
+
+    def _detect(self) -> List[Output]:
+        results = []
+
+        # pylint: disable=attribute-defined-outside-init
+        self.results = []
+        self.visited_all_paths = {}
+
+        for contract in self.compilation_unit.contracts:
+            for function in contract.functions:
+                if (
+                    function.is_implemented
+                    and function.contract_declarer == contract
+                    and function.entry_point
+                ):
+                    function.entry_point.context[self.key] = []
+                    self._detect_multiple_tickets(function, function.entry_point, [])
+
+        for multiple_tickets in self.results:
+            info = ["Multiple retryable tickets created in the same function:\n"]
+
+            for x in multiple_tickets:
+                info += ["\t -", x, "\n"]
+
+            json = self.generate_result(info)
+            results.append(json)
+
+        return results

slither/printers/abstract_printer.py

@@ -20,7 +20,7 @@ class AbstractPrinter(metaclass=abc.ABCMeta):
 
     WIKI = ""
 
-    def __init__(self, slither: "Slither", logger: Logger) -> None:
+    def __init__(self, slither: "Slither", logger: Optional[Logger]) -> None:
         self.slither = slither
         self.contracts = slither.contracts
         self.filename = slither.filename

slither/printers/inheritance/inheritance_graph.py

@@ -100,10 +100,11 @@ def _summary(self, contract):
 
         # 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 += f"{contract.name} -> {contract.immediate_inheritance[0]};\n"
+            immediate_inheritance = contract.immediate_inheritance[0]
+            ret += f"c{contract.id}_{contract.name} -> c{immediate_inheritance.id}_{immediate_inheritance};\n"
         else:
             for i, immediate_inheritance in enumerate(contract.immediate_inheritance):
-                ret += f'{contract.name} -> {immediate_inheritance} [ label="{i + 1}" ];\n'
+                ret += f'c{contract.id}_{contract.name} -> c{immediate_inheritance.id}_{immediate_inheritance} [ label="{i + 1}" ];\n'
 
         # Functions
         visibilities = ["public", "external"]
@@ -151,7 +152,7 @@ def _summary(self, contract):
         indirect_shadowing_information = self._get_indirect_shadowing_information(contract)
 
         # Build the node label
-        ret += f'{contract.name}[shape="box"'
+        ret += f'c{contract.id}_{contract.name}[shape="box"'
         ret += 'label=< <TABLE border="0">'
         ret += f'<TR><TD align="center"><B>{contract.name}</B></TD></TR>'
         if public_functions:

slither/slither.py

@@ -132,6 +132,10 @@ def __init__(self, target: Union[str, CryticCompile], **kwargs) -> None:
         for p in filter_paths:
             self.add_path_to_filter(p)
 
+        include_paths = kwargs.get("include_paths", [])
+        for p in include_paths:
+            self.add_path_to_include(p)
+
         self._exclude_dependencies = kwargs.get("exclude_dependencies", False)
 
         triage_mode = kwargs.get("triage_mode", False)