Merge branch 'cryptomental-feature/rewrite-uninitialized-state-with-ir' into dev

scripts/travis_test.sh

@@ -15,7 +15,7 @@ test_slither(){
     fi
 }
 
-test_slither tests/uninitialized.sol "uninitialized-state" 1
+test_slither tests/uninitialized.sol "uninitialized-state" 4
 test_slither tests/backdoor.sol "backdoor" 1
 test_slither tests/backdoor.sol "suicidal" 1
 test_slither tests/pragma.0.4.24.sol "pragma" 1
@@ -32,7 +32,6 @@ test_slither tests/naming_convention.sol "naming-convention" 10
 test_slither tests/low_level_calls.sol "low-level-calls" 1
 test_slither tests/const_state_variables.sol "const-candidates-state" 2
 
-
 ### Test scripts
 
 python examples/scripts/functions_called.py examples/scripts/functions_called.sol

slither/detectors/variables/uninitialized_state_variables.py

@@ -2,16 +2,20 @@
     Module detecting state uninitialized variables
     Recursively check the called functions
 
-    The heuristic chekcs that:
+    The heuristic checks:
-    - state variables are read or called
+    - state variables including mappings/refs
-    - the variables does not call push (avoid too many FP)
+    - LibraryCalls, InternalCalls, InternalDynamicCalls with storage variables
 
     Only analyze "leaf" contracts (contracts that are not inherited by another contract)
 """
 
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.core.variables.state_variable import StateVariable
+from slither.slithir.variables import ReferenceVariable
+from slither.slithir.operations.assignment import Assignment
 
-from slither.visitors.expression.find_push import FindPush
+from slither.slithir.operations import (OperationWithLValue, Index, Member,
+                                        InternalCall, InternalDynamicCall, LibraryCall)
 
 
 class UninitializedStateVarsDetection(AbstractDetector):
@@ -24,29 +28,28 @@ class UninitializedStateVarsDetection(AbstractDetector):
     IMPACT = DetectorClassification.HIGH
     CONFIDENCE = DetectorClassification.HIGH
 
-    def detect_uninitialized(self, contract):
+    @staticmethod
-        # get all the state variables read by all functions
+    def written_variables(contract):
-        var_read = [f.state_variables_read for f in contract.all_functions_called + contract.modifiers]
+        ret = []
-        # flat list
+        for f in contract.all_functions_called + contract.modifiers:
-        var_read = [item for sublist in var_read for item in sublist]
+            for n in f.nodes:
-        # remove state variable that are initiliazed at contract construction
+                ret += n.state_variables_written
-        var_read = [v for v in var_read if v.uninitialized]
+                for ir in n.irs:
-
+                    if isinstance(ir, LibraryCall) \
-        # get all the state variables written by the functions
+                            or isinstance(ir, InternalCall) \
-        var_written = [f.state_variables_written for f in contract.all_functions_called + contract.modifiers]
+                            or isinstance(ir, InternalDynamicCall):
-        # flat list
+                        idx = 0
-        var_written = [item for sublist in var_written for item in sublist]
+                        for param in ir.function.parameters:
-
+                            if param.location == 'storage':
-        all_push = [f.apply_visitor(FindPush) for f in contract.functions]
+                                ret.append(ir.arguments[idx])
-        # flat list
+                            idx = idx+1
-        all_push = [item for sublist in all_push for item in sublist]
+
+        return ret
 
-        uninitialized_vars = list(set([v for v in var_read if \
+    def detect_uninitialized(self, contract):
-                                       v not in var_written and \
+        written_variables = self.written_variables(contract)
-                                       v not in all_push and \
+        return [(variable, contract.get_functions_reading_from_variable(variable))
-                                       v.type not in contract.using_for]))  # Note: does not handle using X for *
+                for variable in contract.state_variables if variable not in written_variables]
-
-        return [(v, contract.get_functions_reading_from_variable(v)) for v in uninitialized_vars]
 
     def detect(self):
         """ Detect uninitialized state variables
@@ -61,8 +64,8 @@ class UninitializedStateVarsDetection(AbstractDetector):
             for variable, functions in ret:
                 info = "Uninitialized state variable in %s, " % self.filename + \
                        "Contract: %s, Variable: %s, Used in %s" % (c.name,
-                                                               str(variable),
+                                                                   str(variable),
-                                                               [str(f) for f in functions])
+                                                                   [str(f) for f in functions])
                 self.log(info)
 
                 source = [variable.source_mapping]

tests/uninitialized.sol

@@ -9,3 +9,50 @@ contract Uninitialized{
     }
 
 }
+
+
+contract Test {
+    mapping (address => uint) balances;
+    mapping (address => uint) balancesInitialized;
+
+
+    function init() {
+        balancesInitialized[msg.sender] = 0;
+    }
+
+    function use() {
+        // random operation to use the mapping
+        require(balances[msg.sender] == balancesInitialized[msg.sender]);
+    }
+}
+
+library Lib{
+
+    struct MyStruct{
+        uint val;
+    }
+
+    function set(MyStruct storage st, uint v){
+        st.val = 4;
+    }
+
+}
+
+
+contract Test2 {
+    using Lib for Lib.MyStruct;
+
+    Lib.MyStruct st;
+    Lib.MyStruct stInitiliazed;
+    uint v; // v is used as parameter of the lib, but is never init
+
+    function init(){
+        stInitiliazed.set(v);
+    }
+
+    function use(){
+        // random operation to use the structure
+        require(st.val  == stInitiliazed.val);
+    }
+
+}