Merge pull request #2635 from crytic/dev

Sync Master <> Dev

Author: smonicas

.github/workflows/black_auto.yml

@@ -30,12 +30,14 @@ jobs:
         with:
           python-version: 3.8
 
-      - name: Install Python dependencies
-        run: pip install .[dev]
+      - name: Run black
+        uses: psf/black@stable
+        with:
+          options: ""
+          summary: false
+          version: "~= 22.3.0"
 
-      - name: Run linters
-        uses: wearerequired/lint-action@v2
+      - name: Annotate diff changes using reviewdog
+        uses: reviewdog/action-suggester@v1
         with:
-          auto_fix: true
-          black: true
-          black_auto_fix: true
+          tool_name: blackfmt

.github/workflows/ci.yml

@@ -25,26 +25,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: ["ubuntu-latest", "windows-2022"]
+        os: ${{ (github.event_name == 'pull_request' && fromJSON('["ubuntu-latest"]')) || fromJSON('["ubuntu-latest", "windows-2022"]') }}
         python: ${{ (github.event_name == 'pull_request' && fromJSON('["3.8", "3.12"]')) || fromJSON('["3.8", "3.9", "3.10", "3.11", "3.12"]') }}
-        type: ["cli",
-               "dapp",
-               "data_dependency",
-               "path_filtering",
-               # "embark",
-               "erc",
-               # "etherlime",
-               "etherscan",
-               "find_paths",
-               "flat",
-               "interface",
-               "kspec",
-               "printers",
-               # "prop"
-               "simil",
-               "slither_config",
-               "truffle",
-               "upgradability"]
+        type: ${{ (github.event_name == 'pull_request' && fromJSON('["data_dependency", "path_filtering","erc","find_paths","flat","interface", "printers","slither_config","upgradability"]')) || fromJSON('["data_dependency", "path_filtering","erc","find_paths","flat","interface", "printers","slither_config","upgradability", "cli", "dapp", "etherscan", "kspec", "simil", "truffle"]') }}
         exclude:
           # Requires nix
           - os: windows-2022
@@ -67,7 +50,7 @@ jobs:
 
       - name: Set up nix
         if: matrix.type == 'dapp'
-        uses: cachix/install-nix-action@V27
+        uses: cachix/install-nix-action@v30
 
       - name: Set up cachix
         if: matrix.type == 'dapp'

.github/workflows/publish.yml

@@ -44,7 +44,7 @@ jobs:
           path: dist/
 
       - name: publish
-        uses: pypa/gh-action-pypi-publish@v1.9.0
+        uses: pypa/gh-action-pypi-publish@v1.12.3
 
       - name: sign
         uses: sigstore/[email protected]

CODEOWNERS

@@ -1,5 +1,4 @@
-*                            @montyly @0xalpharush @smonicas
-/slither/tools/read_storage/ @0xalpharush
+*                            @montyly @smonicas
 /slither/tools/doctor/       @elopez
 /slither/slithir/            @montyly
 /slither/analyses/           @montyly

Makefile

@@ -6,7 +6,7 @@ TEST_MODULE := tests
 ALL_PY_SRCS := $(shell find $(PY_MODULE) -name '*.py') \
 	$(shell find test -name '*.py')
 
-# Optionally overriden by the user, if they're using a virtual environment manager.
+# Optionally overridden by the user, if they're using a virtual environment manager.
 VENV ?= env
 
 # On Windows, venv scripts/shims are under `Scripts` instead of `bin`.

README.md

@@ -5,7 +5,7 @@
 cd tests/e2e/solc_parsing/test_data/compile/ || exit
 
 # Do not test the evm printer,as it needs a refactoring
-ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,halstead,human-summary,inheritance,inheritance-graph,loc,martin,slithir,slithir-ssa,vars-and-auth,require,variable-order,declaration,ck"
+ALL_PRINTERS="cfg,constructor-calls,contract-summary,data-dependency,echidna,function-id,function-summary,modifiers,call-graph,halstead,human-summary,inheritance,inheritance-graph,loc,martin,slithir,slithir-ssa,vars-and-auth,require,variable-order,declaration,ck,cheatcode"
 
 # Only test 0.5.17 to limit test time
 for file in *0.5.17-compact.zip; do

scripts/ci_test_printers.sh

@@ -8,14 +8,14 @@
     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.4",
+    version="0.11.0",
     packages=find_packages(),
     python_requires=">=3.8",
     install_requires=[
         "packaging",
         "prettytable>=3.10.2",
         "pycryptodome>=3.4.6",
-        "crytic-compile>=0.3.7,<0.4.0",
+        "crytic-compile>=0.3.8,<0.4.0",
         # "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile",
         "web3>=6.20.2, <7",
         "eth-abi>=4.0.0",

setup.py

@@ -86,7 +86,7 @@ def _visit(
             lvalue = refs_lvalues
 
     ret: List[Variable] = []
-    if not node.sons and node.type not in [NodeType.THROW, NodeType.RETURN]:
+    if not node.sons and node.type is not NodeType.THROW:
         ret += [v for v in variables_to_write if v not in variables_written]
 
     # Explore sons if

slither/analyses/write/are_variables_written.py

@@ -46,12 +46,6 @@
 if TYPE_CHECKING:
     from slither.slithir.variables.variable import SlithIRVariable
     from slither.core.compilation_unit import SlitherCompilationUnit
-    from slither.utils.type_helpers import (
-        InternalCallType,
-        HighLevelCallType,
-        LibraryCallType,
-        LowLevelCallType,
-    )
     from slither.core.cfg.scope import Scope
     from slither.core.scope.scope import FileScope
 
@@ -153,11 +147,11 @@ def __init__(
         self._ssa_vars_written: List["SlithIRVariable"] = []
         self._ssa_vars_read: List["SlithIRVariable"] = []
 
-        self._internal_calls: List[Union["Function", "SolidityFunction"]] = []
-        self._solidity_calls: List[SolidityFunction] = []
-        self._high_level_calls: List["HighLevelCallType"] = []  # contains library calls
-        self._library_calls: List["LibraryCallType"] = []
-        self._low_level_calls: List["LowLevelCallType"] = []
+        self._internal_calls: List[InternalCall] = []  # contains solidity calls
+        self._solidity_calls: List[SolidityCall] = []
+        self._high_level_calls: List[Tuple[Contract, HighLevelCall]] = []  # contains library calls
+        self._library_calls: List[LibraryCall] = []
+        self._low_level_calls: List[LowLevelCall] = []
         self._external_calls_as_expressions: List[Expression] = []
         self._internal_calls_as_expressions: List[Expression] = []
         self._irs: List[Operation] = []
@@ -226,8 +220,9 @@ def type(self, new_type: NodeType) -> None:
     @property
     def will_return(self) -> bool:
         if not self.sons and self.type != NodeType.THROW:
-            if SolidityFunction("revert()") not in self.solidity_calls:
-                if SolidityFunction("revert(string)") not in self.solidity_calls:
+            solidity_calls = [ir.function for ir in self.solidity_calls]
+            if SolidityFunction("revert()") not in solidity_calls:
+                if SolidityFunction("revert(string)") not in solidity_calls:
                     return True
         return False
 
@@ -373,44 +368,38 @@ def variables_written_as_expression(self, exprs: List[Expression]) -> None:
     ###################################################################################
 
     @property
-    def internal_calls(self) -> List["InternalCallType"]:
+    def internal_calls(self) -> List[InternalCall]:
         """
-        list(Function or SolidityFunction): List of internal/soldiity function calls
+        list(InternalCall): List of IR operations with internal/solidity function calls
         """
         return list(self._internal_calls)
 
     @property
-    def solidity_calls(self) -> List[SolidityFunction]:
+    def solidity_calls(self) -> List[SolidityCall]:
         """
-        list(SolidityFunction): List of Soldity calls
+        list(SolidityCall): List of IR operations with solidity calls
         """
         return list(self._solidity_calls)
 
     @property
-    def high_level_calls(self) -> List["HighLevelCallType"]:
+    def high_level_calls(self) -> List[HighLevelCall]:
         """
-        list((Contract, Function|Variable)):
-        List of high level calls (external calls).
-        A variable is called in case of call to a public state variable
+        list(HighLevelCall): List of IR operations with high level calls (external calls).
         Include library calls
         """
         return list(self._high_level_calls)
 
     @property
-    def library_calls(self) -> List["LibraryCallType"]:
+    def library_calls(self) -> List[LibraryCall]:
         """
-        list((Contract, Function)):
-        Include library calls
+        list(LibraryCall): List of IR operations with library calls.
         """
         return list(self._library_calls)
 
     @property
-    def low_level_calls(self) -> List["LowLevelCallType"]:
+    def low_level_calls(self) -> List[LowLevelCall]:
         """
-        list((Variable|SolidityVariable, str)): List of low_level call
-        A low level call is defined by
-        - the variable called
-        - the name of the function (call/delegatecall/codecall)
+        list(LowLevelCall): List of IR operations with low_level call
         """
         return list(self._low_level_calls)
 
@@ -529,8 +518,9 @@ def contains_require_or_assert(self) -> bool:
             bool: True if the node has a require or assert call
         """
         return any(
-            c.name in ["require(bool)", "require(bool,string)", "assert(bool)"]
-            for c in self.internal_calls
+            ir.function.name
+            in ["require(bool)", "require(bool,string)", "require(bool,error)", "assert(bool)"]
+            for ir in self.internal_calls
         )
 
     def contains_if(self, include_loop: bool = True) -> bool:
@@ -894,11 +884,11 @@ def _find_read_write_call(self) -> None:  # pylint: disable=too-many-statements
                     self._vars_written.append(var)
 
             if isinstance(ir, InternalCall):
-                self._internal_calls.append(ir.function)
+                self._internal_calls.append(ir)
             if isinstance(ir, SolidityCall):
                 # TODO: consider removing dependancy of solidity_call to internal_call
-                self._solidity_calls.append(ir.function)
-                self._internal_calls.append(ir.function)
+                self._solidity_calls.append(ir)
+                self._internal_calls.append(ir)
             if (
                 isinstance(ir, SolidityCall)
                 and ir.function == SolidityFunction("sstore(uint256,uint256)")
@@ -916,22 +906,22 @@ def _find_read_write_call(self) -> None:  # pylint: disable=too-many-statements
                 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)))
+                self._low_level_calls.append(ir)
             elif isinstance(ir, HighLevelCall) and not isinstance(ir, LibraryCall):
                 # Todo investigate this if condition
                 # It does seem right to compare against a contract
                 # This might need a refactoring
                 if isinstance(ir.destination.type, Contract):
-                    self._high_level_calls.append((ir.destination.type, ir.function))
+                    self._high_level_calls.append((ir.destination.type, ir))
                 elif ir.destination == SolidityVariable("this"):
                     func = self.function
                     # Can't use this in a top level function
                     assert isinstance(func, FunctionContract)
-                    self._high_level_calls.append((func.contract, ir.function))
+                    self._high_level_calls.append((func.contract, ir))
                 else:
                     try:
                         # Todo this part needs more tests and documentation
-                        self._high_level_calls.append((ir.destination.type.type, ir.function))
+                        self._high_level_calls.append((ir.destination.type.type, ir))
                     except AttributeError as error:
                         #  pylint: disable=raise-missing-from
                         raise SlitherException(
@@ -940,8 +930,8 @@ def _find_read_write_call(self) -> None:  # pylint: disable=too-many-statements
             elif isinstance(ir, LibraryCall):
                 assert isinstance(ir.destination, Contract)
                 assert isinstance(ir.function, Function)
-                self._high_level_calls.append((ir.destination, ir.function))
-                self._library_calls.append((ir.destination, ir.function))
+                self._high_level_calls.append((ir.destination, ir))
+                self._library_calls.append(ir)
 
         self._vars_read = list(set(self._vars_read))
         self._state_vars_read = [v for v in self._vars_read if isinstance(v, StateVariable)]

slither/core/cfg/node.py

@@ -73,7 +73,8 @@ def __init__(self, core: "SlitherCore", crytic_compilation_unit: CompilationUnit
         # Memoize
         self._all_state_variables: Optional[Set[StateVariable]] = None
 
-        self._storage_layouts: Dict[str, Dict[str, Tuple[int, int]]] = {}
+        self._persistent_storage_layouts: Dict[str, Dict[str, Tuple[int, int]]] = {}
+        self._transient_storage_layouts: Dict[str, Dict[str, Tuple[int, int]]] = {}
 
         self._contract_with_missing_inheritance: Set[Contract] = set()
 
@@ -297,33 +298,52 @@ def get_scope(self, filename_str: str) -> FileScope:
 
     def compute_storage_layout(self) -> None:
         assert self.is_solidity
+
         for contract in self.contracts_derived:
-            self._storage_layouts[contract.name] = {}
-
-            slot = 0
-            offset = 0
-            for var in contract.stored_state_variables_ordered:
-                assert var.type
-                size, new_slot = var.type.storage_size
-
-                if new_slot:
-                    if offset > 0:
-                        slot += 1
-                        offset = 0
-                elif size + offset > 32:
+            self._compute_storage_layout(contract.name, contract.storage_variables_ordered, False)
+            self._compute_storage_layout(contract.name, contract.transient_variables_ordered, True)
+
+    def _compute_storage_layout(
+        self, contract_name: str, state_variables_ordered: List[StateVariable], is_transient: bool
+    ):
+        if is_transient:
+            self._transient_storage_layouts[contract_name] = {}
+        else:
+            self._persistent_storage_layouts[contract_name] = {}
+
+        slot = 0
+        offset = 0
+        for var in state_variables_ordered:
+            assert var.type
+            size, new_slot = var.type.storage_size
+
+            if new_slot:
+                if offset > 0:
                     slot += 1
                     offset = 0
+            elif size + offset > 32:
+                slot += 1
+                offset = 0
 
-                self._storage_layouts[contract.name][var.canonical_name] = (
+            if is_transient:
+                self._transient_storage_layouts[contract_name][var.canonical_name] = (
                     slot,
                     offset,
                 )
-                if new_slot:
-                    slot += math.ceil(size / 32)
-                else:
-                    offset += size
+            else:
+                self._persistent_storage_layouts[contract_name][var.canonical_name] = (
+                    slot,
+                    offset,
+                )
+
+            if new_slot:
+                slot += math.ceil(size / 32)
+            else:
+                offset += size
 
     def storage_layout_of(self, contract: Contract, var: StateVariable) -> Tuple[int, int]:
-        return self._storage_layouts[contract.name][var.canonical_name]
+        if var.is_stored:
+            return self._persistent_storage_layouts[contract.name][var.canonical_name]
+        return self._transient_storage_layouts[contract.name][var.canonical_name]
 
     # endregion