Merge branch 'dev' into handle-malformed-alias

slither/solc_parsing/declarations/function.py

@@ -308,7 +308,7 @@ class FunctionSolc(CallerContextExpression):
         for node_parser in self._node_to_yulobject.values():
             node_parser.analyze_expressions()
 
-        self._filter_ternary()
+        self._rewrite_ternary_as_if_else()
 
         self._remove_alone_endif()
 
@@ -1336,7 +1336,7 @@ class FunctionSolc(CallerContextExpression):
     ###################################################################################
     ###################################################################################
 
-    def _filter_ternary(self) -> bool:
+    def _rewrite_ternary_as_if_else(self) -> bool:
         ternary_found = True
         updated = False
         while ternary_found:

slither/solc_parsing/declarations/modifier.py

@@ -87,7 +87,7 @@ class ModifierSolc(FunctionSolc):
         for node in self._node_to_nodesolc.values():
             node.analyze_expressions(self)
 
-        self._filter_ternary()
+        self._rewrite_ternary_as_if_else()
         self._remove_alone_endif()
 
         # self._analyze_read_write()

slither/solc_parsing/slither_compilation_unit_solc.py

@@ -24,6 +24,7 @@ from slither.solc_parsing.declarations.function import FunctionSolc
 from slither.solc_parsing.declarations.structure_top_level import StructureTopLevelSolc
 from slither.solc_parsing.exceptions import VariableNotFound
 from slither.solc_parsing.variables.top_level_variable import TopLevelVariableSolc
+from slither.solc_parsing.declarations.caller_context import CallerContextExpression
 
 logging.basicConfig()
 logger = logging.getLogger("SlitherSolcParsing")
@@ -63,7 +64,7 @@ def _handle_import_aliases(
                 )
 
 
-class SlitherCompilationUnitSolc:
+class SlitherCompilationUnitSolc(CallerContextExpression):
     # pylint: disable=no-self-use,too-many-instance-attributes
     def __init__(self, compilation_unit: SlitherCompilationUnit):
         super().__init__()
@@ -101,6 +102,10 @@ class SlitherCompilationUnitSolc:
     def underlying_contract_to_parser(self) -> Dict[Contract, ContractSolc]:
         return self._underlying_contract_to_parser
 
+    @property
+    def slither_parser(self) -> "SlitherCompilationUnitSolc":
+        return self
+
     ###################################################################################
     ###################################################################################
     # region AST

slither/tools/upgradeability/__main__.py

@@ -14,7 +14,10 @@ from slither.exceptions import SlitherException
 from slither.utils.colors import red
 from slither.utils.output import output_to_json
 from slither.tools.upgradeability.checks import all_checks
-from slither.tools.upgradeability.checks.abstract_checks import AbstractCheck
+from slither.tools.upgradeability.checks.abstract_checks import (
+    AbstractCheck,
+    CheckClassification,
+)
 from slither.tools.upgradeability.utils.command_line import (
     output_detectors_json,
     output_wiki,
@@ -27,12 +30,14 @@ logger: logging.Logger = logging.getLogger("Slither")
 logger.setLevel(logging.INFO)
 
 
-def parse_args() -> argparse.Namespace:
+def parse_args(check_classes: List[Type[AbstractCheck]]) -> argparse.Namespace:
     parser = argparse.ArgumentParser(
         description="Slither Upgradeability Checks. For usage information see https://github.com/crytic/slither/wiki/Upgradeability-Checks.",
         usage="slither-check-upgradeability contract.sol ContractName",
     )
 
+    group_checks = parser.add_argument_group("Checks")
+
     parser.add_argument("contract.sol", help="Codebase to analyze")
     parser.add_argument("ContractName", help="Contract name (logic contract)")
 
@@ -51,7 +56,16 @@ def parse_args() -> argparse.Namespace:
         default=False,
     )
 
-    parser.add_argument(
+    group_checks.add_argument(
+        "--detect",
+        help="Comma-separated list of detectors, defaults to all, "
+        f"available detectors: {', '.join(d.ARGUMENT for d in check_classes)}",
+        action="store",
+        dest="detectors_to_run",
+        default="all",
+    )
+
+    group_checks.add_argument(
         "--list-detectors",
         help="List available detectors",
         action=ListDetectors,
@@ -59,6 +73,42 @@ def parse_args() -> argparse.Namespace:
         default=False,
     )
 
+    group_checks.add_argument(
+        "--exclude",
+        help="Comma-separated list of detectors that should be excluded",
+        action="store",
+        dest="detectors_to_exclude",
+        default=None,
+    )
+
+    group_checks.add_argument(
+        "--exclude-informational",
+        help="Exclude informational impact analyses",
+        action="store_true",
+        default=False,
+    )
+
+    group_checks.add_argument(
+        "--exclude-low",
+        help="Exclude low impact analyses",
+        action="store_true",
+        default=False,
+    )
+
+    group_checks.add_argument(
+        "--exclude-medium",
+        help="Exclude medium impact analyses",
+        action="store_true",
+        default=False,
+    )
+
+    group_checks.add_argument(
+        "--exclude-high",
+        help="Exclude high impact analyses",
+        action="store_true",
+        default=False,
+    )
+
     parser.add_argument(
         "--markdown-root",
         help="URL for markdown generation",
@@ -104,6 +154,43 @@ def _get_checks() -> List[Type[AbstractCheck]]:
     return detectors
 
 
+def choose_checks(
+    args: argparse.Namespace, all_check_classes: List[Type[AbstractCheck]]
+) -> List[Type[AbstractCheck]]:
+    detectors_to_run = []
+    detectors = {d.ARGUMENT: d for d in all_check_classes}
+
+    if args.detectors_to_run == "all":
+        detectors_to_run = all_check_classes
+        if args.detectors_to_exclude:
+            detectors_excluded = args.detectors_to_exclude.split(",")
+            for detector in detectors:
+                if detector in detectors_excluded:
+                    detectors_to_run.remove(detectors[detector])
+    else:
+        for detector in args.detectors_to_run.split(","):
+            if detector in detectors:
+                detectors_to_run.append(detectors[detector])
+            else:
+                raise Exception(f"Error: {detector} is not a detector")
+        detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT)
+        return detectors_to_run
+
+    if args.exclude_informational:
+        detectors_to_run = [
+            d for d in detectors_to_run if d.IMPACT != CheckClassification.INFORMATIONAL
+        ]
+    if args.exclude_low:
+        detectors_to_run = [d for d in detectors_to_run if d.IMPACT != CheckClassification.LOW]
+    if args.exclude_medium:
+        detectors_to_run = [d for d in detectors_to_run if d.IMPACT != CheckClassification.MEDIUM]
+    if args.exclude_high:
+        detectors_to_run = [d for d in detectors_to_run if d.IMPACT != CheckClassification.HIGH]
+
+    # detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT)
+    return detectors_to_run
+
+
 class ListDetectors(argparse.Action):  # pylint: disable=too-few-public-methods
     def __call__(
         self, parser: Any, *args: Any, **kwargs: Any
@@ -200,11 +287,11 @@ def main() -> None:
         "detectors": [],
     }
 
-    args = parse_args()
-
+    detectors = _get_checks()
+    args = parse_args(detectors)
+    detectors_to_run = choose_checks(args, detectors)
     v1_filename = vars(args)["contract.sol"]
     number_detectors_run = 0
-    detectors = _get_checks()
     try:
         variable1 = Slither(v1_filename, **vars(args))
 
@@ -219,7 +306,7 @@ def main() -> None:
             return
         v1_contract = v1_contracts[0]
 
-        detectors_results, number_detectors = _checks_on_contract(detectors, v1_contract)
+        detectors_results, number_detectors = _checks_on_contract(detectors_to_run, v1_contract)
         json_results["detectors"] += detectors_results
         number_detectors_run += number_detectors
 
@@ -242,7 +329,7 @@ def main() -> None:
             json_results["proxy-present"] = True
 
             detectors_results, number_detectors = _checks_on_contract_and_proxy(
-                detectors, v1_contract, proxy_contract
+                detectors_to_run, v1_contract, proxy_contract
             )
             json_results["detectors"] += detectors_results
             number_detectors_run += number_detectors
@@ -267,19 +354,19 @@ def main() -> None:
 
             if proxy_contract:
                 detectors_results, _ = _checks_on_contract_and_proxy(
-                    detectors, v2_contract, proxy_contract
+                    detectors_to_run, v2_contract, proxy_contract
                 )
 
                 json_results["detectors"] += detectors_results
 
             detectors_results, number_detectors = _checks_on_contract_update(
-                detectors, v1_contract, v2_contract
+                detectors_to_run, v1_contract, v2_contract
             )
             json_results["detectors"] += detectors_results
             number_detectors_run += number_detectors
 
             # If there is a V2, we run the contract-only check on the V2
-            detectors_results, number_detectors = _checks_on_contract(detectors, v2_contract)
+            detectors_results, number_detectors = _checks_on_contract(detectors_to_run, v2_contract)
             json_results["detectors"] += detectors_results
             number_detectors_run += number_detectors
 

slither/utils/expression_manipulations.py

@@ -23,20 +23,29 @@ from slither.all_exceptions import SlitherException
 
 # pylint: disable=protected-access
 def f_expressions(
-    e: AssignmentOperation, x: Union[Identifier, Literal, MemberAccess, IndexAccess]
+    e: Union[AssignmentOperation, BinaryOperation, TupleExpression],
+    x: Union[Identifier, Literal, MemberAccess, IndexAccess],
 ) -> None:
     e._expressions.append(x)
 
 
-def f_call(e, x):
+def f_call(e: CallExpression, x):
     e._arguments.append(x)
 
 
-def f_expression(e, x):
+def f_call_value(e: CallExpression, x):
+    e._value = x
+
+
+def f_call_gas(e: CallExpression, x):
+    e._gas = x
+
+
+def f_expression(e: Union[TypeConversion, UnaryOperation, MemberAccess], x):
     e._expression = x
 
 
-def f_called(e, x):
+def f_called(e: CallExpression, x):
     e._called = x
 
 
@@ -53,13 +62,20 @@ class SplitTernaryExpression:
             self.condition = None
             self.copy_expression(expression, self.true_expression, self.false_expression)
 
-    def apply_copy(
+    def conditional_not_ahead(
         self,
         next_expr: Expression,
         true_expression: Union[AssignmentOperation, MemberAccess],
         false_expression: Union[AssignmentOperation, MemberAccess],
         f: Callable,
     ) -> bool:
+        # look ahead for parenthetical expression (.. ? .. : ..)
+        if (
+            isinstance(next_expr, TupleExpression)
+            and len(next_expr.expressions) == 1
+            and isinstance(next_expr.expressions[0], ConditionalExpression)
+        ):
+            next_expr = next_expr.expressions[0]
 
         if isinstance(next_expr, ConditionalExpression):
             f(true_expression, copy.copy(next_expr.then_expression))
@@ -71,7 +87,6 @@ class SplitTernaryExpression:
         f(false_expression, copy.copy(next_expr))
         return True
 
-    # pylint: disable=too-many-branches
     def copy_expression(
         self, expression: Expression, true_expression: Expression, false_expression: Expression
     ) -> None:
@@ -87,57 +102,20 @@ class SplitTernaryExpression:
         ):
             return
 
-        # case of lib
-        # (.. ? .. : ..).add
-        if isinstance(expression, MemberAccess):
-            next_expr = expression.expression
-            if self.apply_copy(next_expr, true_expression, false_expression, f_expression):
-                self.copy_expression(
-                    next_expr, true_expression.expression, false_expression.expression
-                )
-
-        elif isinstance(expression, (AssignmentOperation, BinaryOperation, TupleExpression)):
+        if isinstance(expression, (AssignmentOperation, BinaryOperation, TupleExpression)):
             true_expression._expressions = []
             false_expression._expressions = []
-
-            for next_expr in expression.expressions:
-                if isinstance(next_expr, IndexAccess):
-                    # create an index access for each branch
-                    if isinstance(next_expr.expression_right, ConditionalExpression):
-                        next_expr = _handle_ternary_access(
-                            next_expr, true_expression, false_expression
-                        )
-                if self.apply_copy(next_expr, true_expression, false_expression, f_expressions):
-                    # always on last arguments added
-                    self.copy_expression(
-                        next_expr,
-                        true_expression.expressions[-1],
-                        false_expression.expressions[-1],
-                    )
+            self.convert_expressions(expression, true_expression, false_expression)
 
         elif isinstance(expression, CallExpression):
             next_expr = expression.called
+            self.convert_call_expression(expression, next_expr, true_expression, false_expression)
 
-            # case of lib
-            # (.. ? .. : ..).add
-            if self.apply_copy(next_expr, true_expression, false_expression, f_called):
-                self.copy_expression(next_expr, true_expression.called, false_expression.called)
-
-            true_expression._arguments = []
-            false_expression._arguments = []
-
-            for next_expr in expression.arguments:
-                if self.apply_copy(next_expr, true_expression, false_expression, f_call):
-                    # always on last arguments added
-                    self.copy_expression(
-                        next_expr,
-                        true_expression.arguments[-1],
-                        false_expression.arguments[-1],
-                    )
-
-        elif isinstance(expression, (TypeConversion, UnaryOperation)):
+        elif isinstance(expression, (TypeConversion, UnaryOperation, MemberAccess)):
             next_expr = expression.expression
-            if self.apply_copy(next_expr, true_expression, false_expression, f_expression):
+            if self.conditional_not_ahead(
+                next_expr, true_expression, false_expression, f_expression
+            ):
                 self.copy_expression(
                     expression.expression,
                     true_expression.expression,
@@ -149,34 +127,90 @@ class SplitTernaryExpression:
                 f"Ternary operation not handled {expression}({type(expression)})"
             )
 
+    def convert_expressions(
+        self,
+        expression: Union[AssignmentOperation, BinaryOperation, TupleExpression],
+        true_expression: Expression,
+        false_expression: Expression,
+    ) -> None:
+        for next_expr in expression.expressions:
+            # TODO: can we get rid of `NoneType` expressions in `TupleExpression`?
+            # montyly: this might happen with unnamed tuple (ex: (,,,) = f()), but it needs to be checked
+            if next_expr:
+                if isinstance(next_expr, IndexAccess):
+                    self.convert_index_access(next_expr, true_expression, false_expression)
+
+                if self.conditional_not_ahead(
+                    next_expr, true_expression, false_expression, f_expressions
+                ):
+                    # always on last arguments added
+                    self.copy_expression(
+                        next_expr,
+                        true_expression.expressions[-1],
+                        false_expression.expressions[-1],
+                    )
+
+    def convert_index_access(
+        self, next_expr: IndexAccess, true_expression: Expression, false_expression: Expression
+    ) -> None:
+        # create an index access for each branch
+        #  x[if cond ? 1 : 2] -> if cond { x[1] } else { x[2] }
+        for expr in next_expr.expressions:
+            if self.conditional_not_ahead(expr, true_expression, false_expression, f_expressions):
+                self.copy_expression(
+                    expr,
+                    true_expression.expressions[-1],
+                    false_expression.expressions[-1],
+                )
 
-def _handle_ternary_access(
-    next_expr: IndexAccess,
-    true_expression: AssignmentOperation,
-    false_expression: AssignmentOperation,
-):
-    """
-    Conditional ternary accesses are split into two accesses, one true and one false
-    E.g.  x[if cond ? 1 : 2] -> if cond { x[1] } else { x[2] }
-    """
-    true_index_access = IndexAccess(
-        next_expr.expression_left,
-        next_expr.expression_right.then_expression,
-        next_expr.type,
-    )
-    false_index_access = IndexAccess(
-        next_expr.expression_left,
-        next_expr.expression_right.else_expression,
-        next_expr.type,
-    )
-
-    f_expressions(
-        true_expression,
-        true_index_access,
-    )
-    f_expressions(
-        false_expression,
-        false_index_access,
-    )
-
-    return next_expr.expression_right
+    def convert_call_expression(
+        self,
+        expression: CallExpression,
+        next_expr: Expression,
+        true_expression: Expression,
+        false_expression: Expression,
+    ) -> None:
+        # case of lib
+        # (.. ? .. : ..).add
+        if self.conditional_not_ahead(next_expr, true_expression, false_expression, f_called):
+            self.copy_expression(next_expr, true_expression.called, false_expression.called)
+
+        # In order to handle ternaries in both call options, gas and value, we return early if the
+        # conditional is not ahead to rewrite both ternaries (see `_rewrite_ternary_as_if_else`).
+        if expression.call_gas:
+            # case of (..).func{gas: .. ? .. : ..}()
+            next_expr = expression.call_gas
+            if self.conditional_not_ahead(next_expr, true_expression, false_expression, f_call_gas):
+                self.copy_expression(
+                    next_expr,
+                    true_expression.call_gas,
+                    false_expression.call_gas,
+                )
+            else:
+                return
+
+        if expression.call_value:
+            # case of (..).func{value: .. ? .. : ..}()
+            next_expr = expression.call_value
+            if self.conditional_not_ahead(
+                next_expr, true_expression, false_expression, f_call_value
+            ):
+                self.copy_expression(
+                    next_expr,
+                    true_expression.call_value,
+                    false_expression.call_value,
+                )
+            else:
+                return
+
+        true_expression._arguments = []
+        false_expression._arguments = []
+
+        for expr in expression.arguments:
+            if self.conditional_not_ahead(expr, true_expression, false_expression, f_call):
+                # always on last arguments added
+                self.copy_expression(
+                    expr,
+                    true_expression.arguments[-1],
+                    false_expression.arguments[-1],
+                )

tests/ast-parsing/expected/top-level-struct-0.8.0.sol-0.8.0-compact.json

@@ -0,0 +1,3 @@
+{
+  "BaseContract": {}
+}

tests/ast-parsing/top-level-struct-0.8.0.sol

@@ -0,0 +1,18 @@
+struct my_struct {
+    uint[][] a; // works fine
+    uint[][3] b; // works fine
+    uint[3][] c; // fails
+    uint[3][3] d; // fails
+    uint[2**20] e; // works fine
+}
+contract BaseContract{
+    struct my_struct_2 {
+        uint[][] f; // works fine
+        uint[][3] g; // works fine
+        uint[3][] h; // works fine
+        uint[3][3] i; // works fine
+        uint[2**20] j; // works fine
+    }
+    
+    uint[3][] k; // works fine
+}

tests/slithir/ternary_expressions.sol

@@ -1,3 +1,7 @@
+interface Test {
+    function test() external payable returns (uint);
+    function testTuple() external payable returns (uint, uint);
+}
 contract C {
     // TODO
     // 1) support variable declarations
@@ -21,4 +25,18 @@ contract C {
     function d(bool cond, bytes calldata x) external {
         bytes1 a = x[cond ? 1 : 2];
     }
+
+    function e(address one, address two) public {
+        uint x = Test(one).test{value: msg.sender == two ? 1 : 2, gas: true ? 2 : gasleft()}();
+    }
+
+    // Parenthetical expression
+    function f(address one, address two) public {
+        uint x = Test(one).test{value: msg.sender == two ? 1 : 2, gas: true ? (1 == 1 ? 1 : 2) : gasleft()}();
+    }
+
+    // Unused tuple variable
+    function g(address one) public {
+        (, uint x) = Test(one).testTuple();
+    }
 }

tests/slithir/test_ternary_expressions.py

@@ -9,10 +9,10 @@ def test_ternary_conversions() -> None:
     slither = Slither("./tests/slithir/ternary_expressions.sol")
     for contract in slither.contracts:
         for function in contract.functions:
+            vars_declared = 0
+            vars_assigned = 0
             for node in function.nodes:
                 if node.type in [NodeType.IF, NodeType.IFLOOP]:
-                    vars_declared = 0
-                    vars_assigned = 0
 
                     # Iterate over true and false son
                     for inner_node in node.sons:
@@ -31,7 +31,7 @@ def test_ternary_conversions() -> None:
                             if isinstance(ir, Assignment):
                                 vars_assigned += 1
 
-                    assert vars_declared == vars_assigned
+            assert vars_declared == vars_assigned
 
 
 if __name__ == "__main__":

tests/test_ast_parsing.py

@@ -425,6 +425,7 @@ ALL_TESTS = [
     Test("free_functions/library_constant_function_collision.sol", ["0.8.12"]),
     Test("ternary-with-max.sol", ["0.8.15"]),
     Test("library_event-0.8.16.sol", ["0.8.16"]),
+    Test("top-level-struct-0.8.0.sol", ["0.8.0"]),
     Test("complex_imports/import_aliases_issue_1319/test.sol", ["0.5.12"]),
 ]
 # create the output folder if needed