diff --git a/scripts/travis_test.sh b/scripts/travis_test.sh index 42ab6881f..b3378aaad 100755 --- a/scripts/travis_test.sh +++ b/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 diff --git a/slither/detectors/variables/uninitialized_state_variables.py b/slither/detectors/variables/uninitialized_state_variables.py index 717425e26..8ec652782 100644 --- a/slither/detectors/variables/uninitialized_state_variables.py +++ b/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: - - state variables are read or called - - the variables does not call push (avoid too many FP) + The heuristic checks: + - state variables including mappings/refs + - 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): - # get all the state variables read by all functions - var_read = [f.state_variables_read for f in contract.all_functions_called + contract.modifiers] - # flat list - var_read = [item for sublist in var_read for item in sublist] - # remove state variable that are initiliazed at contract construction - var_read = [v for v in var_read if v.uninitialized] - - # get all the state variables written by the functions - var_written = [f.state_variables_written for f in contract.all_functions_called + contract.modifiers] - # flat list - var_written = [item for sublist in var_written for item in sublist] - - all_push = [f.apply_visitor(FindPush) for f in contract.functions] - # flat list - all_push = [item for sublist in all_push for item in sublist] + @staticmethod + def written_variables(contract): + ret = [] + for f in contract.all_functions_called + contract.modifiers: + for n in f.nodes: + ret += n.state_variables_written + for ir in n.irs: + if isinstance(ir, LibraryCall) \ + or isinstance(ir, InternalCall) \ + or isinstance(ir, InternalDynamicCall): + idx = 0 + for param in ir.function.parameters: + if param.location == 'storage': + ret.append(ir.arguments[idx]) + idx = idx+1 + + return ret - uninitialized_vars = list(set([v for v in var_read if \ - v not in var_written and \ - v not in all_push and \ - v.type not in contract.using_for])) # Note: does not handle using X for * - - return [(v, contract.get_functions_reading_from_variable(v)) for v in uninitialized_vars] + def detect_uninitialized(self, contract): + written_variables = self.written_variables(contract) + return [(variable, contract.get_functions_reading_from_variable(variable)) + for variable in contract.state_variables if variable not in written_variables] 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(f) for f in functions]) + str(variable), + [str(f) for f in functions]) self.log(info) source = [variable.source_mapping] diff --git a/tests/uninitialized.sol b/tests/uninitialized.sol index ff3f1addc..2305483e6 100644 --- a/tests/uninitialized.sol +++ b/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); + } + +}