Merge pull request #1762 from crytic/fix-new-contract-reads

include salt in NewContract op, add test infra

.github/workflows/features.yml

@@ -50,6 +50,7 @@ jobs:
           pytest tests/test_features.py
           pytest tests/test_constant_folding.py
           pytest tests/slithir/test_ternary_expressions.py
+          pytest tests/slithir/test_operation_reads.py
           pytest tests/test_functions_ids.py
           pytest tests/test_function.py
           pytest tests/test_source_mapping.py

slither/slithir/operations/high_level_call.py

@@ -76,7 +76,7 @@ class HighLevelCall(Call, OperationWithLValue):
     def read(self) -> List[SourceMapping]:
         all_read = [self.destination, self.call_gas, self.call_value] + self._unroll(self.arguments)
         # remove None
-        return [x for x in all_read if x] + [self.destination]
+        return [x for x in all_read if x]
 
     @property
     def destination(self) -> SourceMapping:

slither/slithir/operations/new_contract.py

@@ -52,7 +52,9 @@ class NewContract(Call, OperationWithLValue):  # pylint: disable=too-many-instan
 
     @property
     def read(self) -> List[Any]:
-        return self._unroll(self.arguments)
+        all_read = [self.call_salt, self.call_value] + self._unroll(self.arguments)
+        # remove None
+        return [x for x in all_read if x]
 
     @property
     def contract_created(self) -> Contract:

tests/slithir/operation_reads.sol

@@ -0,0 +1,17 @@
+
+contract Placeholder {
+    constructor() payable {}
+}
+
+contract NewContract {
+    bytes32 internal constant state_variable_read = bytes32(0);
+
+    function readAllStateVariables() external {
+        new Placeholder{salt: state_variable_read} ();
+    }
+
+    function readAllLocalVariables() external {
+        bytes32 local_variable_read = bytes32(0);
+        new Placeholder{salt: local_variable_read} ();
+    }
+}

tests/slithir/test_operation_reads.py

@@ -0,0 +1,49 @@
+from collections import namedtuple
+from slither import Slither
+from slither.slithir.operations import Operation, NewContract
+
+
+def check_num_local_vars_read(function, slithir_op: Operation, num_reads_expected: int):
+    for node in function.nodes:
+        for operation in node.irs:
+            if isinstance(operation, slithir_op):
+                assert len(operation.read) == num_reads_expected
+                assert len(node.local_variables_read) == num_reads_expected
+
+
+def check_num_states_vars_read(function, slithir_op: Operation, num_reads_expected: int):
+    for node in function.nodes:
+        for operation in node.irs:
+            if isinstance(operation, slithir_op):
+                assert len(operation.read) == num_reads_expected
+                assert len(node.state_variables_read) == num_reads_expected
+
+
+OperationTest = namedtuple("OperationTest", "contract_name slithir_op")
+
+OPERATION_TEST = [OperationTest("NewContract", NewContract)]
+
+
+def test_operation_reads() -> None:
+    """
+    Every slithir operation has its own contract and reads all local and state variables in readAllLocalVariables and readAllStateVariables, respectively.
+    """
+    slither = Slither("./tests/slithir/operation_reads.sol")
+
+    for op_test in OPERATION_TEST:
+        print(op_test)
+        available = slither.get_contract_from_name(op_test.contract_name)
+        assert len(available) == 1
+        target = available[0]
+
+        num_state_variables = len(target.state_variables_ordered)
+        state_function = target.get_function_from_signature("readAllStateVariables()")
+        check_num_states_vars_read(state_function, op_test.slithir_op, num_state_variables)
+
+        local_function = target.get_function_from_signature("readAllLocalVariables()")
+        num_local_vars = len(local_function.local_variables)
+        check_num_local_vars_read(local_function, op_test.slithir_op, num_local_vars)
+
+
+if __name__ == "__main__":
+    test_operation_reads()