Merge branch 'dev' into dev-fix-aor-array

.github/ISSUE_TEMPLATE/bug_report.yml

@@ -3,9 +3,13 @@ body:
   - 
     attributes: 
       value: |
-          Please check the issues tab to avoid duplicates.
+          Please check the issues tab to avoid duplicates, and 
+          confirm that the bug exists on the latest release (upgrade
+          by running `python3 -m pip install --upgrade slither-analyzer`).
+          
           If you are having difficulty installing slither,
           please head over to the "Discussions" page.
+          
           Thanks for taking the time to fill out this bug report!
     type: markdown
   - 
@@ -17,7 +21,7 @@ body:
       required: true
   - 
     attributes: 
-      description: "It can be a github repo, etherscan link, or code snippet."
+      description: "It can be a github repo (preferred), etherscan link, or code snippet."
       label: "Code example to reproduce the issue:"
       placeholder: "`contract A {}`\n"
     id: reproduce

.github/scripts/tool_test_runner.sh

@@ -2,11 +2,11 @@
 
 # used to pass --cov=$path and --cov-append to pytest
 if [ "$1" != "" ]; then
-    pytest "$1" tests/tools/read-storage/test_read_storage.py
+    pytest "$1" tests/tools
     status_code=$?
     python -m coverage report
 else
-    pytest tests/tools/read-storage/test_read_storage.py
+    pytest tests/tools
     status_code=$?
 fi
 

.github/workflows/docker.yml

@@ -47,7 +47,7 @@ jobs:
           password: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Docker Build and Push
-        uses: docker/build-push-action@v5
+        uses: docker/build-push-action@v6
         with:
           platforms: linux/amd64,linux/arm64/v8,linux/arm/v7
           target: final

.github/workflows/publish.yml

@@ -44,7 +44,7 @@ jobs:
           path: dist/
 
       - name: publish
-        uses: pypa/gh-action-pypi-publish@v1.8.14
+        uses: pypa/gh-action-pypi-publish@v1.9.0
 
       - name: sign
         uses: sigstore/gh-action-sigstore-python@v2.1.1

Dockerfile

@@ -3,6 +3,7 @@ FROM ubuntu:jammy AS python-wheels
 RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
     gcc \
     git \
+    make \
     python3-dev \
     python3-pip \
   && rm -rf /var/lib/apt/lists/*

slither/__main__.py

@@ -239,6 +239,7 @@ def choose_detectors(
             set(detectors_to_run), args.detectors_to_include, detectors
         )
 
+    detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT)
     return detectors_to_run
 
 
@@ -255,7 +256,6 @@ def __include_detectors(
         else:
             raise ValueError(f"Error: {detector} is not a detector")
 
-    detectors_to_run = sorted(detectors_to_run, key=lambda x: x.IMPACT)
     return detectors_to_run
 
 

slither/detectors/attributes/incorrect_solc.py

@@ -71,7 +71,7 @@ Consider using the latest version of Solidity for testing."""
         if op and op not in [">", ">=", "^"]:
             return self.LESS_THAN_TXT
         version_number = ".".join(version[2:])
-        if version_number in bugs_by_version:
+        if version_number in bugs_by_version and len(bugs_by_version[version_number]):
             bugs = "\n".join([f"\t- {bug}" for bug in bugs_by_version[version_number]])
             return self.BUGGY_VERSION_TXT + f"\n{bugs}"
         return None

slither/detectors/functions/arbitrary_send_eth.py

@@ -30,6 +30,7 @@ from slither.slithir.operations import (
     SolidityCall,
     Transfer,
 )
+from slither.core.variables.state_variable import StateVariable
 
 # pylint: disable=too-many-nested-blocks,too-many-branches
 from slither.utils.output import Output
@@ -67,6 +68,8 @@ def arbitrary_send(func: Function) -> Union[bool, List[Node]]:
                     continue
                 if ir.call_value == SolidityVariableComposed("msg.value"):
                     continue
+                if isinstance(ir.destination, StateVariable) and ir.destination.is_immutable:
+                    continue
                 if is_dependent(
                     ir.call_value,
                     SolidityVariableComposed("msg.value"),

slither/detectors/functions/dead_code.py

@@ -25,7 +25,7 @@ class DeadCode(AbstractDetector):
     WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#dead-code"
 
     WIKI_TITLE = "Dead-code"
-    WIKI_DESCRIPTION = "Functions that are not sued."
+    WIKI_DESCRIPTION = "Functions that are not used."
 
     # region wiki_exploit_scenario
     WIKI_EXPLOIT_SCENARIO = """
@@ -71,9 +71,10 @@ contract Contract{
                 continue
             if isinstance(function, FunctionContract) and (
                 function.contract_declarer.is_from_dependency()
+                or function.contract_declarer.is_library
             ):
                 continue
-            # Continue if the functon is not implemented because it means the contract is abstract
+            # Continue if the function is not implemented because it means the contract is abstract
             if not function.is_implemented:
                 continue
             info: DETECTOR_INFO = [function, " is never used and should be removed\n"]

slither/tools/mutator/__main__.py

@@ -6,7 +6,7 @@ import shutil
 import sys
 import time
 from pathlib import Path
-from typing import Type, List, Any, Optional
+from typing import Type, List, Any, Optional, Union
 from crytic_compile import cryticparser
 from slither import Slither
 from slither.tools.mutator.utils.testing_generated_mutant import run_test_cmd
@@ -116,7 +116,7 @@ def parse_args() -> argparse.Namespace:
     return parser.parse_args()
 
 
-def _get_mutators(mutators_list: List[str] | None) -> List[Type[AbstractMutator]]:
+def _get_mutators(mutators_list: Union[List[str], None]) -> List[Type[AbstractMutator]]:
     detectors_ = [getattr(all_mutators, name) for name in dir(all_mutators)]
     if mutators_list is not None:
         detectors = [

slither/tools/mutator/mutators/LIR.py

@@ -31,7 +31,7 @@ class LIR(AbstractMutator):  # pylint: disable=too-few-public-methods
                         literal_replacements.append(variable.type.max)  # append data type max value
                         if str(variable.type).startswith("uint"):
                             literal_replacements.append("1")
-                        elif str(variable.type).startswith("uint"):
+                        elif str(variable.type).startswith("int"):
                             literal_replacements.append("-1")
                     # Get the string
                     start = variable.source_mapping.start
@@ -63,7 +63,7 @@ class LIR(AbstractMutator):  # pylint: disable=too-few-public-methods
                         literal_replacements.append(variable.type.max)
                         if str(variable.type).startswith("uint"):
                             literal_replacements.append("1")
-                        elif str(variable.type).startswith("uint"):
+                        elif str(variable.type).startswith("int"):
                             literal_replacements.append("-1")
                     start = variable.source_mapping.start
                     stop = start + variable.source_mapping.length

slither/tools/mutator/mutators/abstract_mutator.py

@@ -1,7 +1,7 @@
 import abc
 import logging
 from pathlib import Path
-from typing import Optional, Dict, Tuple, List
+from typing import Optional, Dict, Tuple, List, Union
 from slither.core.compilation_unit import SlitherCompilationUnit
 from slither.formatters.utils.patches import apply_patch, create_diff
 from slither.tools.mutator.utils.testing_generated_mutant import test_patch
@@ -27,7 +27,7 @@ class AbstractMutator(
         testing_command: str,
         testing_directory: str,
         contract_instance: Contract,
-        solc_remappings: str | None,
+        solc_remappings: Union[str, None],
         verbose: bool,
         very_verbose: bool,
         output_folder: Path,
@@ -81,7 +81,7 @@ class AbstractMutator(
         (all_patches) = self._mutate()
         if "patches" not in all_patches:
             logger.debug("No patches found by %s", self.NAME)
-            return ([0, 0, 0], [0, 0, 0], self.dont_mutate_line)
+            return [0, 0, 0], [0, 0, 0], self.dont_mutate_line
 
         for file in all_patches["patches"]:  # Note: This should only loop over a single file
             original_txt = self.slither.source_code[file].encode("utf8")
@@ -146,4 +146,4 @@ class AbstractMutator(
                             f"Found {self.uncaught_mutant_counts[2]} uncaught tweak mutants so far (out of {self.total_mutant_counts[2]} that compile)"
                         )
 
-        return (self.total_mutant_counts, self.uncaught_mutant_counts, self.dont_mutate_line)
+        return self.total_mutant_counts, self.uncaught_mutant_counts, self.dont_mutate_line

slither/tools/mutator/utils/file_handling.py

@@ -111,7 +111,7 @@ def get_sol_file_list(codebase: Path, ignore_paths: Union[List[str], None]) -> L
     # if input is folder
     if codebase.is_dir():
         for file_name in codebase.rglob("*.sol"):
-            if not any(part in ignore_paths for part in file_name.parts):
+            if file_name.is_file() and not any(part in ignore_paths for part in file_name.parts):
                 sol_file_list.append(file_name.as_posix())
 
     return sol_file_list

slither/tools/mutator/utils/testing_generated_mutant.py

@@ -22,7 +22,12 @@ def compile_generated_mutant(file_path: str, mappings: str) -> bool:
         return False
 
 
-def run_test_cmd(cmd: str, timeout: int | None, target_file: str | None, verbose: bool) -> bool:
+def run_test_cmd(
+    cmd: str,
+    timeout: Union[int, None] = None,
+    target_file: Union[str, None] = None,
+    verbose: bool = False,
+) -> bool:
     """
     function to run codebase tests
     returns: boolean whether the tests passed or not

tests/e2e/detectors/snapshots/detectors__detector_ArbitrarySendEth_0_6_11_arbitrary_send_eth_sol__0.txt

@@ -1,8 +1,8 @@
-Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#19-21) sends eth to arbitrary user
+Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#16-18) sends eth to arbitrary user
 	Dangerous calls:
-	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#20)
+	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#17)
 
-Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#11-13) sends eth to arbitrary user
+Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#24-26) sends eth to arbitrary user
 	Dangerous calls:
-	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#12)
+	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol#25)
 

tests/e2e/detectors/snapshots/detectors__detector_ArbitrarySendEth_0_7_6_arbitrary_send_eth_sol__0.txt

@@ -1,8 +1,8 @@
-Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#11-13) sends eth to arbitrary user
+Test.direct() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#16-18) sends eth to arbitrary user
 	Dangerous calls:
-	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#12)
+	- msg.sender.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#17)
 
-Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#19-21) sends eth to arbitrary user
+Test.indirect() (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#24-26) sends eth to arbitrary user
 	Dangerous calls:
-	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#20)
+	- destination.send(address(this).balance) (tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol#25)
 

tests/e2e/detectors/test_data/arbitrary-send-eth/0.6.11/arbitrary_send_eth.sol

@@ -1,13 +1,18 @@
 contract Test{
 
     address payable destination;
-
+    address payable immutable destination_imm;
     mapping (address => uint) balances;
 
     constructor() public{
+        destination_imm = payable(msg.sender);
         balances[msg.sender] = 0;
     }
 
+    function send_immutable() public{
+        destination_imm.send(address(this).balance);
+    }
+
     function direct() public{
         msg.sender.send(address(this).balance);
     }

tests/e2e/detectors/test_data/arbitrary-send-eth/0.7.6/arbitrary_send_eth.sol

@@ -1,13 +1,18 @@
 contract Test{
 
     address payable destination;
-
+    address payable immutable destination_imm;
     mapping (address => uint) balances;
 
     constructor() public{
+        destination_imm = payable(msg.sender);
         balances[msg.sender] = 0;
     }
 
+    function send_immutable() public{
+        destination_imm.send(address(this).balance);
+    }
+
     function direct() public{
         msg.sender.send(address(this).balance);
     }

tests/tools/mutator/test_data/test_source_unit/README.md

@@ -0,0 +1,7 @@
+# Counter
+
+Init using :
+
+```shell
+forge install --no-commit --no-git .
+```

tests/tools/mutator/test_data/test_source_unit/foundry.toml

@@ -0,0 +1,7 @@
+[profile.default]
+src = 'src'
+out = 'out'
+libs = ['lib']
+solc = "0.8.15"
+
+# See more config options https://github.com/foundry-rs/foundry/tree/master/config

tests/tools/mutator/test_data/test_source_unit/script/Counter.s.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.13;
+
+import {Script, console} from "forge-std/Script.sol";
+
+contract CounterScript is Script {
+    function setUp() public {}
+
+    function run() public {
+        vm.broadcast();
+    }
+}

tests/tools/mutator/test_data/test_source_unit/src/Counter.sol

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

tests/tools/mutator/test_data/test_source_unit/test/Counter.t.sol

@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.15;
+
+import {Test, console} from "forge-std/Test.sol";
+import {Counter} from "../src/Counter.sol";
+
+contract CounterTest is Test {
+    Counter public counter;
+
+    function setUp() public {
+        counter = new Counter();
+        counter.setNumber(0);
+    }
+
+    function test_Increment() public {
+        counter.increment();
+        assertEq(counter.number(), 1);
+    }
+
+    function testFuzz_SetNumber(uint256 x) public {
+        counter.setNumber(x);
+        assertEq(counter.number(), x);
+    }
+}

tests/tools/mutator/test_mutator.py

@@ -0,0 +1,133 @@
+import argparse
+from contextlib import contextmanager
+import os
+from pathlib import Path
+import shutil
+import subprocess
+import tempfile
+from unittest import mock
+
+import pytest
+from slither import Slither
+from slither.tools.mutator.__main__ import _get_mutators, main
+from slither.tools.mutator.utils.testing_generated_mutant import run_test_cmd
+from slither.tools.mutator.utils.file_handling import get_sol_file_list, backup_source_file
+
+
+TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data"
+
+foundry_available = shutil.which("forge") is not None
+project_ready = Path(TEST_DATA_DIR, "test_source_unit/lib/forge-std").exists()
+
+
+@contextmanager
+def change_directory(new_dir):
+    original_dir = os.getcwd()
+    os.chdir(new_dir)
+    try:
+        yield
+    finally:
+        os.chdir(original_dir)
+
+
+def test_get_mutators():
+
+    mutators = _get_mutators(None)
+    assert mutators
+
+    mutators = _get_mutators(["ASOR"])
+    assert len(mutators) == 1
+    assert mutators[0].NAME == "ASOR"
+
+    mutators = _get_mutators(["ASOR", "NotExisiting"])
+    assert len(mutators) == 1
+
+
+@mock.patch(
+    "argparse.ArgumentParser.parse_args",
+    return_value=argparse.Namespace(
+        test_cmd="forge test",
+        test_dir=None,
+        ignore_dirs="lib,mutation_campaign",
+        output_dir=None,
+        timeout=None,
+        solc_remaps="forge-std=./lib/forge-std",
+        verbose=None,
+        very_verbose=None,
+        mutators_to_run=None,
+        comprehensive=None,
+        codebase=(TEST_DATA_DIR / "test_source_unit" / "src" / "Counter.sol").as_posix(),
+        contract_names="Counter",
+    ),
+)
+@pytest.mark.skip(reason="Slow test")
+def test_mutator(mock_args, solc_binary_path):  # pylint: disable=unused-argument
+
+    with change_directory(TEST_DATA_DIR / "test_source_unit"):
+        main()
+
+
+def test_backup_source_file(solc_binary_path):
+    solc_path = solc_binary_path("0.8.15")
+
+    file_path = (TEST_DATA_DIR / "test_source_unit" / "src" / "Counter.sol").as_posix()
+    sl = Slither(file_path, solc=solc_path)
+
+    with tempfile.TemporaryDirectory() as directory:
+        files_dict = backup_source_file(sl.source_code, Path(directory))
+
+        assert len(files_dict) == 1
+        assert Path(files_dict[file_path]).exists()
+
+
+@pytest.mark.skipif(
+    not foundry_available or not project_ready, reason="requires Foundry and project setup"
+)
+def test_get_sol_file_list():
+
+    project_directory = TEST_DATA_DIR / "test_source_unit"
+
+    files = get_sol_file_list(project_directory, None)
+
+    assert len(files) == 46
+
+    files = get_sol_file_list(project_directory, ["lib"])
+    assert len(files) == 3
+
+    files = get_sol_file_list(project_directory, ["lib", "script"])
+    assert len(files) == 2
+
+    files = get_sol_file_list(project_directory / "src" / "Counter.sol", None)
+    assert len(files) == 1
+
+    (project_directory / "test.sol").mkdir()
+    files = get_sol_file_list(project_directory, None)
+    assert all("test.sol" not in file for file in files)
+    (project_directory / "test.sol").rmdir()
+
+
+@pytest.mark.skipif(
+    not foundry_available or not project_ready, reason="requires Foundry and project setup"
+)
+def test_run_test(caplog):
+    with change_directory(TEST_DATA_DIR / "test_source_unit"):
+        result = run_test_cmd("forge test", timeout=None, target_file=None, verbose=True)
+        assert result
+        assert not caplog.records
+
+        # Failed command
+        result = run_test_cmd("forge non-test", timeout=None, target_file=None, verbose=True)
+        assert not result
+        assert caplog.records
+
+
+def test_run_tests_timeout(caplog, monkeypatch):
+    def mock_run(*args, **kwargs):
+        raise subprocess.TimeoutExpired(cmd=args[0], timeout=kwargs.get("timeout"))
+
+    monkeypatch.setattr(subprocess, "run", mock_run)
+
+    with change_directory(TEST_DATA_DIR / "test_source_unit"):
+        result = run_test_cmd("forge test", timeout=1)
+        assert not result
+        assert "Tests took too long" in caplog.messages[0]