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

README.md

@@ -196,13 +196,12 @@ Num | Detector | What it Detects | Impact | Confidence
 85 | `costly-loop` | [Costly operations in a loop](https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop) | Informational | Medium
 86 | `dead-code` | [Functions that are not used](https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code) | Informational | Medium
 87 | `reentrancy-unlimited-gas` | [Reentrancy vulnerabilities through send and transfer](https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-4) | Informational | Medium
-88 | `similar-names` | [Variable names are too similar](https://github.com/crytic/slither/wiki/Detector-Documentation#variable-names-too-similar) | Informational | Medium
-89 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium
-90 | `cache-array-length` | [Detects `for` loops that use `length` member of some storage array in their loop condition and don't modify it.](https://github.com/crytic/slither/wiki/Detector-Documentation#cache-array-length) | Optimization | High
-91 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Optimization | High
-92 | `external-function` | [Public function that could be declared external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external) | Optimization | High
-93 | `immutable-states` | [State variables that could be declared immutable](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable) | Optimization | High
-94 | `var-read-using-this` | [Contract reads its own variable using `this`](https://github.com/crytic/slither/wiki/Detector-Documentation#public-variable-read-in-external-context) | Optimization | High
+88 | `too-many-digits` | [Conformance to numeric notation best practices](https://github.com/crytic/slither/wiki/Detector-Documentation#too-many-digits) | Informational | Medium
+89 | `cache-array-length` | [Detects `for` loops that use `length` member of some storage array in their loop condition and don't modify it.](https://github.com/crytic/slither/wiki/Detector-Documentation#cache-array-length) | Optimization | High
+90 | `constable-states` | [State variables that could be declared constant](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant) | Optimization | High
+91 | `external-function` | [Public function that could be declared external](https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external) | Optimization | High
+92 | `immutable-states` | [State variables that could be declared immutable](https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable) | Optimization | High
+93 | `var-read-using-this` | [Contract reads its own variable using `this`](https://github.com/crytic/slither/wiki/Detector-Documentation#public-variable-read-in-external-context) | Optimization | High
 
 For more information, see
 

setup.py

@@ -8,7 +8,7 @@
     description="Slither is a Solidity and Vyper static analysis framework written in Python 3.",
     url="https://github.com/crytic/slither",
     author="Trail of Bits",
-    version="0.10.2",
+    version="0.10.3",
     packages=find_packages(),
     python_requires=">=3.8",
     install_requires=[

slither/core/source_mapping/source_mapping.py

@@ -4,6 +4,7 @@
 from Crypto.Hash import SHA1
 from crytic_compile.utils.naming import Filename
 from slither.core.context.context import Context
+from slither.exceptions import SlitherException
 
 if TYPE_CHECKING:
     from slither.core.compilation_unit import SlitherCompilationUnit
@@ -136,9 +137,20 @@ def _compute_line(
     start_line, starting_column = compilation_unit.core.crytic_compile.get_line_from_offset(
         filename, start
     )
-    end_line, ending_column = compilation_unit.core.crytic_compile.get_line_from_offset(
-        filename, start + length
-    )
+    try:
+        end_line, ending_column = compilation_unit.core.crytic_compile.get_line_from_offset(
+            filename, start + length
+        )
+    except KeyError:
+        # This error may occur when the build is not synchronised with the source code on disk.
+        # See the GitHub issue https://github.com/crytic/slither/issues/2296
+        msg = f"""The source code appears to be out of sync with the build artifacts on disk.
+        This discrepancy can occur after recent modifications to {filename.short}. To resolve this
+        issue, consider executing the clean command of the build system (e.g. forge clean).
+        """
+        # We still re-raise the exception as a SlitherException here
+        raise SlitherException(msg) from None
+
     return list(range(start_line, end_line + 1)), starting_column, ending_column
 
 

slither/detectors/attributes/constant_pragma.py

@@ -36,21 +36,19 @@ def _detect(self) -> List[Output]:
         for pragma in self.compilation_unit.pragma_directives:
             if pragma.is_solidity_version:
                 if pragma.version not in pragma_directives_by_version:
-                    pragma_directives_by_version[
-                        pragma.version
-                    ] = f"\t\t- {str(pragma.source_mapping)}\n"
+                    pragma_directives_by_version[pragma.version] = [pragma]
                 else:
-                    pragma_directives_by_version[
-                        pragma.version
-                    ] += f"\t\t- {str(pragma.source_mapping)}\n"
+                    pragma_directives_by_version[pragma.version].append(pragma)
 
         versions = list(pragma_directives_by_version.keys())
         if len(versions) > 1:
             info: DETECTOR_INFO = [f"{len(versions)} different versions of Solidity are used:\n"]
 
             for version in versions:
-                pragma = pragma_directives_by_version[version]
-                info += [f"\t- Version constraint {version} is used by:\n {pragma}"]
+                pragmas = pragma_directives_by_version[version]
+                info += [f"\t- Version constraint {version} is used by:\n"]
+                for pragma in pragmas:
+                    info += ["\t\t-", pragma, "\n"]
 
             res = self.generate_result(info)
 

slither/detectors/attributes/incorrect_solc.py

@@ -115,16 +115,18 @@ def _detect(self) -> List[Output]:
                 continue
 
             if p.version in disallowed_pragmas and reason in disallowed_pragmas[p.version]:
-                disallowed_pragmas[p.version][reason] += f"\t- {str(p.source_mapping)}\n"
+                disallowed_pragmas[p.version][reason].append(p)
             else:
-                disallowed_pragmas[p.version] = {reason: f"\t- {str(p.source_mapping)}\n"}
+                disallowed_pragmas[p.version] = {reason: [p]}
 
         # If we found any disallowed pragmas, we output our findings.
         if len(disallowed_pragmas.keys()):
             for p, reasons in disallowed_pragmas.items():
                 info: DETECTOR_INFO = []
-                for r, v in reasons.items():
-                    info += [f"Version constraint {p} {r}.\n It is used by:\n{v}"]
+                for r, vers in reasons.items():
+                    info += [f"Version constraint {p} {r}.\nIt is used by:\n"]
+                    for ver in vers:
+                        info += ["\t- ", ver, "\n"]
 
                 json = self.generate_result(info)
 

slither/detectors/statements/unused_import.py

@@ -1,5 +1,6 @@
 from typing import List
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification, Output
+from slither.core.scope.scope import FileScope
 
 # pylint: disable=protected-access,too-many-nested-blocks
 class UnusedImport(AbstractDetector):
@@ -30,26 +31,71 @@ class UnusedImport(AbstractDetector):
         "Remove the unused import. If the import is needed later, it can be added back."
     )
 
-    def _detect(self) -> List[Output]:
+    @staticmethod
+    def _is_import_container(scope: FileScope) -> bool:  # pylint: disable=too-many-branches
+        """
+        Returns True if a given file (provided as a `FileScope` object) contains only `import` directives (and pragmas).
+        Such a file doesn't need the imports it contains, but its purpose is to aggregate certain correlated imports.
+        """
+        for c in scope.contracts.values():
+            if c.file_scope == scope:
+                return False
+        for err in scope.custom_errors:
+            if err.file_scope == scope:
+                return False
+        for en in scope.enums.values():
+            if en.file_scope == scope:
+                return False
+        for f in scope.functions:
+            if f.file_scope == scope:
+                return False
+        for st in scope.structures.values():
+            if st.file_scope == scope:
+                return False
+        for ct in scope.type_aliases.values():
+            if ct.source_mapping and ct.source_mapping.filename == scope.filename:
+                return False
+        for uf in scope.using_for_directives:
+            if uf.file_scope == scope:
+                return False
+        for v in scope.variables.values():
+            if v.file_scope == scope:
+                return False
+        return True
+
+    def _detect(self) -> List[Output]:  # pylint: disable=too-many-branches
         results: List[Output] = []
         # This is computed lazily and then memoized so we need to trigger the computation.
         self.slither._compute_offsets_to_ref_impl_decl()
 
         for unit in self.slither.compilation_units:
-            for filename, scope in unit.scopes.items():
-                unused = []
-                for i in scope.imports:
+            for filename, current_scope in unit.scopes.items():
+                # Skip files that are dependencies
+                if unit.crytic_compile.is_dependency(filename.absolute):
+                    continue
+
+                unused_list = []
+                for i in current_scope.imports:
                     # `scope.imports` contains all transitive imports so we need to filter out imports not explicitly imported in the file.
                     # Otherwise, we would recommend removing an import that is used by a leaf contract and cause compilation errors.
-                    if i.scope != scope:
+                    if i.scope != current_scope:
+                        continue
+
+                    # If a scope doesn't define any contract, function, etc., it is an import container.
+                    # The second case accounts for importing from an import container as a reference will only be in the definition's file.
+                    if self._is_import_container(i.scope) or self._is_import_container(
+                        unit.get_scope(i.filename)
+                    ):
                         continue
 
-                    import_path = self.slither.crytic_compile.filename_lookup(i.filename)
+                    imported_path = self.slither.crytic_compile.filename_lookup(i.filename)
 
                     use_found = False
                     # Search through all references to the imported file
-                    for _, refs in self.slither._offset_to_references[import_path].items():
-                        for ref in refs:
+                    for _, refs_to_imported_path in self.slither._offset_to_references[
+                        imported_path
+                    ].items():
+                        for ref in refs_to_imported_path:
                             # If there is a reference in this file to the imported file, it is used.
                             if ref.filename == filename:
                                 use_found = True
@@ -59,17 +105,15 @@ def _detect(self) -> List[Output]:
                             break
 
                     if not use_found:
-                        unused.append(f"{i.source_mapping.content} ({i.source_mapping})")
-
-                if len(unused) > 0:
-                    unused_list = "\n\t-" + "\n\t-".join(unused)
-
-                    results.append(
-                        self.generate_result(
-                            [
-                                f"The following unused import(s) in {filename.used} should be removed: {unused_list}\n",
-                            ]
-                        )
-                    )
+                        unused_list.append(f"{i.source_mapping.content} ({i.source_mapping})")
+
+                if len(unused_list) > 0:
+                    info = [
+                        f"The following unused import(s) in {filename.used} should be removed:",
+                    ]
+                    for unused in unused_list:
+                        info += ["\n\t-", unused, "\n"]
+
+                    results.append(self.generate_result(info))
 
         return results

slither/solc_parsing/expressions/expression_parsing.py

@@ -501,7 +501,9 @@ def parse_expression(expression: Dict, caller_context: CallerContextExpression)
                 referenced_declaration = expression["attributes"]["referencedDeclaration"]
 
         if t:
-            found = re.findall(r"[struct|enum|function|modifier] \(([\[\] ()a-zA-Z0-9\.,_]*)\)", t)
+            found = re.findall(
+                r"(?:struct|enum|function|modifier) \(([\[\] ()a-zA-Z0-9\.,_]*)\)", t
+            )
             assert len(found) <= 1
             if found:
                 value = value + "(" + found[0] + ")"

tests/e2e/compilation/test_data/test_change/README.md

@@ -0,0 +1,3 @@
+# Foundry
+
+To init this test case, run `forge install --no-commit --no-git foundry-rs/forge-std`

tests/e2e/compilation/test_data/test_change/foundry.toml

@@ -0,0 +1,6 @@
+[profile.default]
+src = "src"
+out = "out"
+libs = ["lib"]
+
+# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

tests/e2e/compilation/test_data/test_change/src/Counter.sol

@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.13;
+
+contract Counter {
+    uint256 public number;
+
+    function setNumber(uint256 newNumber) public {
+        number = newNumber;
+    }
+
+    //START
+    function increment() public {
+        number++;
+    }
+    //END
+}

tests/e2e/compilation/test_diagnostic.py

@@ -0,0 +1,40 @@
+from pathlib import Path
+import shutil
+import re
+
+import pytest
+
+from slither import Slither
+from slither.exceptions import SlitherException
+
+
+TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data"
+
+foundry_available = shutil.which("forge") is not None
+project_ready = Path(TEST_DATA_DIR, "test_change/lib/forge-std").exists()
+
+
+@pytest.mark.skipif(
+    not foundry_available or not project_ready, reason="requires Foundry and project setup"
+)
+def test_diagnostic():
+
+    test_file_directory = TEST_DATA_DIR / "test_change"
+
+    sl = Slither(test_file_directory.as_posix())
+    assert len(sl.compilation_units) == 1
+
+    counter_file = test_file_directory / "src" / "Counter.sol"
+    shutil.copy(counter_file, counter_file.with_suffix(".bak"))
+
+    with counter_file.open("r") as file:
+        content = file.read()
+
+    with counter_file.open("w") as file:
+        file.write(re.sub(r"//START.*?//END\n?", "", content, flags=re.DOTALL))
+
+    with pytest.raises(SlitherException):
+        Slither(test_file_directory.as_posix(), ignore_compile=True)
+
+    # Restore the original counter so the test is idempotent
+    Path(counter_file.with_suffix(".bak")).rename(counter_file)

tests/e2e/detectors/snapshots/detectors__detector_ConstantPragma_0_4_25_pragma_0_4_25_sol__0.txt

@@ -1,6 +1,6 @@
 2 different versions of Solidity are used:
 	- Version constraint ^0.4.25 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.4.25/pragma.0.4.25.sol#1
+		-^0.4.25 (tests/e2e/detectors/test_data/pragma/0.4.25/pragma.0.4.25.sol#1)
 	- Version constraint ^0.4.24 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.4.25/pragma.0.4.24.sol#1
+		-^0.4.24 (tests/e2e/detectors/test_data/pragma/0.4.25/pragma.0.4.24.sol#1)
 

tests/e2e/detectors/snapshots/detectors__detector_ConstantPragma_0_5_16_pragma_0_5_16_sol__0.txt

@@ -1,6 +1,6 @@
 2 different versions of Solidity are used:
 	- Version constraint ^0.5.16 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.5.16/pragma.0.5.16.sol#1
+		-^0.5.16 (tests/e2e/detectors/test_data/pragma/0.5.16/pragma.0.5.16.sol#1)
 	- Version constraint ^0.5.15 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.5.16/pragma.0.5.15.sol#1
+		-^0.5.15 (tests/e2e/detectors/test_data/pragma/0.5.16/pragma.0.5.15.sol#1)
 

tests/e2e/detectors/snapshots/detectors__detector_ConstantPragma_0_6_11_pragma_0_6_11_sol__0.txt

@@ -1,6 +1,6 @@
 2 different versions of Solidity are used:
 	- Version constraint ^0.6.11 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.6.11/pragma.0.6.11.sol#1
+		-^0.6.11 (tests/e2e/detectors/test_data/pragma/0.6.11/pragma.0.6.11.sol#1)
 	- Version constraint ^0.6.10 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.6.11/pragma.0.6.10.sol#1
+		-^0.6.10 (tests/e2e/detectors/test_data/pragma/0.6.11/pragma.0.6.10.sol#1)
 

tests/e2e/detectors/snapshots/detectors__detector_ConstantPragma_0_7_6_pragma_0_7_6_sol__0.txt

@@ -1,6 +1,6 @@
 2 different versions of Solidity are used:
 	- Version constraint ^0.7.6 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.7.6/pragma.0.7.6.sol#1
+		-^0.7.6 (tests/e2e/detectors/test_data/pragma/0.7.6/pragma.0.7.6.sol#1)
 	- Version constraint ^0.7.5 is used by:
- 		- tests/e2e/detectors/test_data/pragma/0.7.6/pragma.0.7.5.sol#1
+		-^0.7.5 (tests/e2e/detectors/test_data/pragma/0.7.6/pragma.0.7.5.sol#1)
 

tests/e2e/detectors/snapshots/detectors__detector_IncorrectSolc_0_4_25_static_sol__0.txt

@@ -1,5 +1,3 @@
-solc-0.4.25 is an outdated solc version. Use a more recent version (at least 0.8.0), if possible.
-
 Version constraint 0.4.25 contains known severe issues (https://solidity.readthedocs.io/en/latest/bugs.html)
 	- DirtyBytesArrayToStorage
 	- ABIDecodeTwoDimensionalArrayMemory
@@ -16,6 +14,8 @@ Version constraint 0.4.25 contains known severe issues (https://solidity.readthe
 	- UninitializedFunctionPointerInConstructor_0.4.x
 	- IncorrectEventSignatureInLibraries_0.4.x
 	- ABIEncoderV2PackedStorage_0.4.x.
- It is used by:
-	- tests/e2e/detectors/test_data/solc-version/0.4.25/static.sol#1
+It is used by:
+	- 0.4.25 (tests/e2e/detectors/test_data/solc-version/0.4.25/static.sol#1)
+
+solc-0.4.25 is an outdated solc version. Use a more recent version (at least 0.8.0), if possible.
 

tests/e2e/detectors/snapshots/detectors__detector_IncorrectSolc_0_5_14_static_sol__0.txt

@@ -13,8 +13,8 @@ Version constraint 0.5.14 contains known severe issues (https://solidity.readthe
 	- privateCanBeOverridden
 	- YulOptimizerRedundantAssignmentBreakContinue0.5
 	- ABIEncoderV2LoopYulOptimizer.
- It is used by:
-	- tests/e2e/detectors/test_data/solc-version/0.5.14/static.sol#1
+It is used by:
+	- 0.5.14 (tests/e2e/detectors/test_data/solc-version/0.5.14/static.sol#1)
 
 solc-0.5.14 is an outdated solc version. Use a more recent version (at least 0.8.0), if possible.
 

tests/e2e/detectors/snapshots/detectors__detector_IncorrectSolc_0_5_16_dynamic_1_sol__0.txt

@@ -12,8 +12,8 @@ Version constraint ^0.5.15 contains known severe issues (https://solidity.readth
 	- MemoryArrayCreationOverflow
 	- privateCanBeOverridden
 	- YulOptimizerRedundantAssignmentBreakContinue0.5.
- It is used by:
-	- tests/e2e/detectors/test_data/solc-version/0.5.16/dynamic_1.sol#1
+It is used by:
+	- ^0.5.15 (tests/e2e/detectors/test_data/solc-version/0.5.16/dynamic_1.sol#1)
 
 solc-0.5.16 is an outdated solc version. Use a more recent version (at least 0.8.0), if possible.
 

tests/e2e/detectors/snapshots/detectors__detector_IncorrectSolc_0_5_16_dynamic_2_sol__0.txt

@@ -14,8 +14,8 @@ Version constraint >=0.5.0<0.6.0 contains known severe issues (https://solidity.
 	- UninitializedFunctionPointerInConstructor
 	- IncorrectEventSignatureInLibraries
 	- ABIEncoderV2PackedStorage.
- It is used by:
-	- tests/e2e/detectors/test_data/solc-version/0.5.16/dynamic_2.sol#1
+It is used by:
+	- >=0.5.0<0.6.0 (tests/e2e/detectors/test_data/solc-version/0.5.16/dynamic_2.sol#1)
 
 solc-0.5.16 is an outdated solc version. Use a more recent version (at least 0.8.0), if possible.