Merge branch 'dev' into fix/ir-assignment-2266

.coderabbit.yaml

@@ -0,0 +1,25 @@
+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+language: "en"
+early_access: false
+knowledge_base:
+  learnings:
+    scope: auto
+  issues:
+    scope: global
+reviews:
+  profile: "chill"
+  request_changes_workflow: false
+  high_level_summary: true
+  poem: false
+  review_status: true
+  collapse_walkthrough: true
+  auto_review:
+    enabled: true
+    ignore_title_keywords:
+      - "WIP"
+      - "DO NOT MERGE"
+    drafts: false
+    base_branches:
+      - dev
+chat:
+  auto_reply: true

plugin_example/setup.py

@@ -1,14 +1,14 @@
 from setuptools import setup, find_packages
 
 setup(
-    name="slither-my-plugins",
+    name="slither_my_plugin",
     description="This is an example of detectors and printers to Slither.",
     url="https://github.com/trailofbits/slither-plugins",
     author="Trail of Bits",
     version="0.0",
     packages=find_packages(),
     python_requires=">=3.8",
-    install_requires=["slither-analyzer==0.1"],
+    install_requires=["slither-analyzer>=0.6.0"],
     entry_points={
         "slither_analyzer.plugin": "slither my-plugin=slither_my_plugin:make_plugin",
     },

plugin_example/slither_my_plugin/detectors/example.py

@@ -11,12 +11,12 @@ class Example(AbstractDetector):  # pylint: disable=too-few-public-methods
     IMPACT = DetectorClassification.HIGH
     CONFIDENCE = DetectorClassification.HIGH
 
-    WIKI = ""
+    WIKI = "https://www.example.com/#example-detector"
 
-    WIKI_TITLE = ""
-    WIKI_DESCRIPTION = ""
-    WIKI_EXPLOIT_SCENARIO = ""
-    WIKI_RECOMMENDATION = ""
+    WIKI_TITLE = "example detector"
+    WIKI_DESCRIPTION = "This is an example detector that always generates a finding"
+    WIKI_EXPLOIT_SCENARIO = "Scenario goes here"
+    WIKI_RECOMMENDATION = "Customize the detector"
 
     def _detect(self):
 

slither/__main__.py

@@ -14,7 +14,7 @@ from importlib import metadata
 from typing import Tuple, Optional, List, Dict, Type, Union, Any, Sequence
 
 
-from crytic_compile import cryticparser, CryticCompile
+from crytic_compile import cryticparser, CryticCompile, InvalidCompilation
 from crytic_compile.platform.standard import generate_standard_export
 from crytic_compile.platform.etherscan import SUPPORTED_NETWORK
 from crytic_compile import compile_all, is_supported
@@ -93,7 +93,13 @@ def process_all(
     detector_classes: List[Type[AbstractDetector]],
     printer_classes: List[Type[AbstractPrinter]],
 ) -> Tuple[List[Slither], List[Dict], List[Output], int]:
-    compilations = compile_all(target, **vars(args))
+
+    try:
+        compilations = compile_all(target, **vars(args))
+    except InvalidCompilation:
+        logger.error("Unable to compile all targets.")
+        sys.exit(2)
+
     slither_instances = []
     results_detectors = []
     results_printers = []

slither/core/declarations/function.py

@@ -1593,7 +1593,7 @@ class Function(SourceMapping, metaclass=ABCMeta):  # pylint: disable=too-many-pu
         write_var = [x for x in write_var if x]
         write_var = [item for sublist in write_var for item in sublist]
         write_var = list(set(write_var))
-        # Remove dupplicate if they share the same string representation
+        # Remove duplicate if they share the same string representation
         write_var = [
             next(obj)
             for i, obj in groupby(sorted(write_var, key=lambda x: str(x)), lambda x: str(x))
@@ -1604,7 +1604,7 @@ class Function(SourceMapping, metaclass=ABCMeta):  # pylint: disable=too-many-pu
         write_var = [x for x in write_var if x]
         write_var = [item for sublist in write_var for item in sublist]
         write_var = list(set(write_var))
-        # Remove dupplicate if they share the same string representation
+        # Remove duplicate if they share the same string representation
         write_var = [
             next(obj)
             for i, obj in groupby(sorted(write_var, key=lambda x: str(x)), lambda x: str(x))
@@ -1614,7 +1614,7 @@ class Function(SourceMapping, metaclass=ABCMeta):  # pylint: disable=too-many-pu
         read_var = [x.variables_read_as_expression for x in self.nodes]
         read_var = [x for x in read_var if x]
         read_var = [item for sublist in read_var for item in sublist]
-        # Remove dupplicate if they share the same string representation
+        # Remove duplicate if they share the same string representation
         read_var = [
             next(obj)
             for i, obj in groupby(sorted(read_var, key=lambda x: str(x)), lambda x: str(x))
@@ -1624,7 +1624,7 @@ class Function(SourceMapping, metaclass=ABCMeta):  # pylint: disable=too-many-pu
         read_var = [x.variables_read for x in self.nodes]
         read_var = [x for x in read_var if x]
         read_var = [item for sublist in read_var for item in sublist]
-        # Remove dupplicate if they share the same string representation
+        # Remove duplicate if they share the same string representation
         read_var = [
             next(obj)
             for i, obj in groupby(sorted(read_var, key=lambda x: str(x)), lambda x: str(x))

slither/detectors/compiler_bugs/reused_base_constructor.py

@@ -35,7 +35,7 @@ class ReusedBaseConstructor(AbstractDetector):
     ARGUMENT = "reused-constructor"
     HELP = "Reused base constructor"
     IMPACT = DetectorClassification.MEDIUM
-    # The confidence is medium, because prior Solidity 0.4.22, we cant differentiate
+    # The confidence is medium, because prior Solidity 0.4.22, we can't differentiate
     # contract C is A() {
     # to
     # contract C is A {

slither/detectors/naming_convention/naming_convention.py

@@ -16,7 +16,7 @@ class NamingConvention(AbstractDetector):
 
     Exceptions:
     - Allow constant variables name/symbol/decimals to be lowercase (ERC20)
-    - Allow '_' at the beggining of the mixed_case match for private variables and unused parameters
+    - Allow '_' at the beginning of the mixed_case match for private variables and unused parameters
     - Ignore echidna properties (functions with names starting 'echidna_' or 'crytic_'
     """
 

slither/detectors/statements/divide_before_multiply.py

@@ -80,7 +80,7 @@ def _explore(
         for ir in node.irs:
             if isinstance(ir, Assignment):
                 if ir.rvalue in divisions:
-                    # Avoid dupplicate. We dont use set so we keep the order of the nodes
+                    # Avoid duplicate. We dont use set so we keep the order of the nodes
                     if node not in divisions[ir.rvalue]:  # type: ignore
                         divisions[ir.lvalue] = divisions[ir.rvalue] + [node]  # type: ignore
                     else:
@@ -94,7 +94,7 @@ def _explore(
                 nodes = []
                 for r in mul_arguments:
                     if not isinstance(r, Constant) and (r in divisions):
-                        # Dont add node already present to avoid dupplicate
+                        # Dont add node already present to avoid duplicate
                         # We dont use set to keep the order of the nodes
                         if node in divisions[r]:
                             nodes += [n for n in divisions[r] if n not in nodes]

slither/detectors/variables/unused_state_variables.py

@@ -46,8 +46,18 @@ def detect_unused(contract: Contract) -> Optional[List[StateVariable]]:
     variables_used = [item for sublist in variables_used for item in sublist]
     variables_used = list(set(variables_used + array_candidates))
 
+    # If the contract is abstract, only consider private variables as other visibilities may be used in dependencies
+    if contract.is_abstract:
+        return [
+            x
+            for x in contract.state_variables
+            if x not in variables_used and x.visibility == "private"
+        ]
+
     # Return the variables unused that are not public
-    return [x for x in contract.variables if x not in variables_used and x.visibility != "public"]
+    return [
+        x for x in contract.state_variables if x not in variables_used and x.visibility != "public"
+    ]
 
 
 class UnusedStateVars(AbstractDetector):
@@ -71,7 +81,7 @@ class UnusedStateVars(AbstractDetector):
         """Detect unused state variables"""
         results = []
         for c in self.compilation_unit.contracts_derived:
-            if c.is_signature_only():
+            if c.is_signature_only() or c.is_library:
                 continue
             unusedVars = detect_unused(c)
             if unusedVars:

slither/printers/summary/evm.py

@@ -1,6 +1,8 @@
 """
     Module printing evm mapping of the contract
 """
+import logging
+
 from slither.printers.abstract_printer import AbstractPrinter
 from slither.analyses.evm import (
     generate_source_to_evm_ins_mapping,
@@ -9,6 +11,9 @@ from slither.analyses.evm import (
 from slither.utils.colors import blue, green, magenta, red
 
 
+logger: logging.Logger = logging.getLogger("EVMPrinter")
+
+
 def _extract_evm_info(slither):
     """
     Extract evm information for all derived contracts using evm_cfg_builder
@@ -24,6 +29,16 @@ def _extract_evm_info(slither):
         contract_bytecode_runtime = contract.file_scope.bytecode_runtime(
             contract.compilation_unit.crytic_compile_compilation_unit, contract.name
         )
+
+        if not contract_bytecode_runtime:
+            logger.info(
+                "Contract %s (abstract: %r) has no bytecode runtime, skipping. ",
+                contract.name,
+                contract.is_abstract,
+            )
+            evm_info["empty", contract.name] = True
+            continue
+
         contract_srcmap_runtime = contract.file_scope.srcmap_runtime(
             contract.compilation_unit.crytic_compile_compilation_unit, contract.name
         )
@@ -80,6 +95,10 @@ class PrinterEVM(AbstractPrinter):
         for contract in self.slither.contracts_derived:
             txt += blue(f"Contract {contract.name}\n")
 
+            if evm_info.get(("empty", contract.name), False):
+                txt += "\tempty contract\n"
+                continue
+
             contract_file = self.slither.source_code[
                 contract.source_mapping.filename.absolute
             ].encode("utf-8")

slither/slithir/convert.py

@@ -482,7 +482,7 @@ def propagate_type_and_convert_call(result: List[Operation], node: "Node") -> Li
             call_data.remove(ins.variable)
 
         if isinstance(ins, Argument):
-            # In case of dupplicate arguments we overwrite the value
+            # In case of duplicate arguments we overwrite the value
             # This can happen because of addr.call.value(1).value(2)
             if ins.get_type() in [ArgumentType.GAS]:
                 calls_gas[ins.call_id] = ins.argument

slither/solc_parsing/declarations/contract.py

@@ -593,7 +593,7 @@ class ContractSolc(CallerContextExpression):
     def analyze_constant_state_variables(self) -> None:
         for var_parser in self._variables_parser:
             if var_parser.underlying_variable.is_constant:
-                # cant parse constant expression based on function calls
+                # can't parse constant expression based on function calls
                 try:
                     var_parser.analyze(self)
                 except (VariableNotFound, KeyError) as e:

slither/tools/kspec_coverage/analysis.py

@@ -59,7 +59,7 @@ def _get_all_covered_kspec_functions(target: str) -> Set[Tuple[str, str]]:
 def _get_slither_functions(
     slither: SlitherCompilationUnit,
 ) -> Dict[Tuple[str, str], Union[FunctionContract, StateVariable]]:
-    # Use contract == contract_declarer to avoid dupplicate
+    # Use contract == contract_declarer to avoid duplicate
     all_functions_declared: List[Union[FunctionContract, StateVariable]] = [
         f
         for f in slither.functions

slither/utils/command_line.py

@@ -75,13 +75,6 @@ defaults_flag_in_config = {
     **DEFAULTS_FLAG_IN_CONFIG_CRYTIC_COMPILE,
 }
 
-deprecated_flags = {
-    "fail_pedantic": True,
-    "fail_low": False,
-    "fail_medium": False,
-    "fail_high": False,
-}
-
 
 def read_config_file(args: argparse.Namespace) -> None:
     # No config file was provided as an argument
@@ -98,12 +91,6 @@ def read_config_file(args: argparse.Namespace) -> None:
             with open(args.config_file, encoding="utf8") as f:
                 config = json.load(f)
                 for key, elem in config.items():
-                    if key in deprecated_flags:
-                        logger.info(
-                            yellow(f"{args.config_file} has a deprecated key: {key} : {elem}")
-                        )
-                        migrate_config_options(args, key, elem)
-                        continue
                     if key not in defaults_flag_in_config:
                         logger.info(
                             yellow(f"{args.config_file} has an unknown key: {key} : {elem}")
@@ -118,28 +105,6 @@ def read_config_file(args: argparse.Namespace) -> None:
         logger.error(yellow("Falling back to the default settings..."))
 
 
-def migrate_config_options(args: argparse.Namespace, key: str, elem):
-    if key.startswith("fail_") and getattr(args, "fail_on") == defaults_flag_in_config["fail_on"]:
-        if key == "fail_pedantic":
-            pedantic_setting = elem
-            fail_on = FailOnLevel.PEDANTIC if pedantic_setting else FailOnLevel.NONE
-            setattr(args, "fail_on", fail_on)
-            logger.info(f"Migrating fail_pedantic: {pedantic_setting} as fail_on: {fail_on.value}")
-        elif key == "fail_low" and elem is True:
-            logger.info("Migrating fail_low: true -> fail_on: low")
-            setattr(args, "fail_on", FailOnLevel.LOW)
-
-        elif key == "fail_medium" and elem is True:
-            logger.info("Migrating fail_medium: true -> fail_on: medium")
-            setattr(args, "fail_on", FailOnLevel.MEDIUM)
-
-        elif key == "fail_high" and elem is True:
-            logger.info("Migrating fail_high: true -> fail_on: high")
-            setattr(args, "fail_on", FailOnLevel.HIGH)
-        else:
-            logger.warning(yellow(f"Key {key} was deprecated but no migration was provided"))
-
-
 def output_to_markdown(
     detector_classes: List[Type[AbstractDetector]],
     printer_classes: List[Type[AbstractPrinter]],

tests/e2e/config/test_path_filtering/slither.config.json

@@ -2,6 +2,6 @@
     "detectors_to_run": "all",
     "exclude_informational": true,
     "exclude_low": true,
-    "fail_pedantic": false,
+    "fail_on": "none",
     "filter_paths": "libs|src/ReentrancyMock.sol"
 }

tests/e2e/detectors/snapshots/detectors__detector_UnusedStateVars_0_7_6_unused_state_sol__0.txt

@@ -4,5 +4,9 @@ A.unused4 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#7)
 
 A.unused2 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#5) is never used in B (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#11-16)
 
+H.bad1 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#38) is never used in I (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#41-46)
+
+F.bad1 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#27) is never used in F (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#26-33)
+
 A.unused3 (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#6) is never used in B (tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol#11-16)
 

tests/e2e/detectors/test_data/unused-state/0.7.6/unused_state.sol

@@ -14,3 +14,32 @@ contract B is A{
         used = address(0);
     }
 }
+
+library C {
+    uint internal constant good = 0x00; // other contract can access this constant
+    function c() public pure returns (uint){
+        return 0;
+    }
+    
+}
+
+abstract contract F {
+    uint private bad1 = 0x00;
+    uint private good1 = 0x00;
+    uint internal good2 = 0x00;
+    function use() external returns (uint){
+        return good1;
+    }
+}
+
+abstract contract H {
+    uint private good1 = 0x00;
+    uint internal good2 = 0x00;
+    uint internal bad1 = 0x00;
+}
+
+contract I is H {
+    function use2() external returns (uint){
+        return good2;
+    }
+}

tests/e2e/test_cli.py

@@ -0,0 +1,20 @@
+import sys
+import tempfile
+import pytest
+
+from slither.__main__ import main_impl
+
+
+def test_cli_exit_on_invalid_compilation_file(caplog):
+
+    with tempfile.NamedTemporaryFile("w") as f:
+        f.write("pragma solidity ^0.10000.0;")
+
+        sys.argv = ["slither", f.name]
+        with pytest.raises(SystemExit) as pytest_wrapped_e:
+            main_impl([], [])
+
+    assert pytest_wrapped_e.type == SystemExit
+    assert pytest_wrapped_e.value.code == 2
+
+    assert caplog.record_tuples[0] == ("Slither", 40, "Unable to compile all targets.")

tests/unit/core/test_source_mapping.py

@@ -58,7 +58,7 @@ def test_source_mapping_inheritance(solc_binary_path, solc_version):
     assert {(x.start, x.end) for x in slither.offset_to_implementations(file, 203)} == {(193, 230)}
 
     # Offset 93 is the call to f() in A.test()
-    # This can lead to three differents functions, depending on the current contract's context
+    # This can lead to three different functions, depending on the current contract's context
     functions = slither.offset_to_objects(file, 93)
     print(functions)
     assert len(functions) == 3

tests/unit/slithir/test_data/ternary_expressions.sol

@@ -6,7 +6,7 @@ contract C {
     // TODO
     // 1) support variable declarations
     //uint min = 1 > 0 ? 1 : 2;
-    // 2) suppory ternary index range access
+    // 2) support ternary index range access
     // function e(bool cond, bytes calldata x) external {
     //     bytes memory a = x[cond ? 1 : 2 :];
     // }

tests/unit/slithir/test_ssa_generation.py

@@ -502,7 +502,7 @@ def test_storage_refers_to(slither_from_solidity_source):
     When a phi-node is created, ensure refers_to is propagated to the phi-node.
     Assignments also propagate refers_to.
     Whenever a ReferenceVariable is the destination of an assignment (e.g. s.v = 10)
-    below, create additional versions of the variables it refers to record that a a
+    below, create additional versions of the variables it refers to record that a
     write was made. In the current implementation, this is referenced by phis.
     """
     source = """
@@ -550,7 +550,7 @@ def test_storage_refers_to(slither_from_solidity_source):
         entryphi = [x for x in phinodes if x.lvalue in sphi.lvalue.refers_to]
         assert len(entryphi) == 2
 
-        # The remaining two phis are the ones recording that write through ReferenceVariable occured
+        # The remaining two phis are the ones recording that write through ReferenceVariable occurred
         for ephi in entryphi:
             phinodes.remove(ephi)
         phinodes.remove(sphi)