Improve constable-states result (remove FP)

slither/detectors/variables/possible_const_state_variables.py

@@ -4,6 +4,9 @@
 
 from slither.core.solidity_types.elementary_type import ElementaryType
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.visitors.expression.export_values import ExportValues
+from slither.core.declarations.solidity_variables import SolidityFunction
+from slither.core.variables.state_variable import StateVariable
 
 class ConstCandidateStateVars(AbstractDetector):
     """
@@ -24,6 +27,37 @@ class ConstCandidateStateVars(AbstractDetector):
     def _valid_candidate(v):
         return isinstance(v.type, ElementaryType) and not v.is_constant
 
+    # https://solidity.readthedocs.io/en/v0.5.2/contracts.html#constant-state-variables
+    valid_solidity_function = [SolidityFunction('keccak256()'),
+                               SolidityFunction('keccak256(bytes)'),
+                               SolidityFunction('sha256()'),
+                               SolidityFunction('sha256(bytes)'),
+                               SolidityFunction('ripemd160()'),
+                               SolidityFunction('ripemd160(bytes)'),
+                               SolidityFunction('ecrecover(bytes32,uint8,bytes32,bytes32)'),
+                               SolidityFunction('addmod(uint256,uint256,uint256)'),
+                               SolidityFunction('mulmod(uint256,uint256,uint256)')]
+
+    @staticmethod
+    def _is_constant_var(v):
+        if isinstance(v, StateVariable):
+            return v.is_constant
+        return False
+
+    def _constant_initial_expression(self, v):
+        if not v.expression:
+            return True
+
+        export = ExportValues(v.expression)
+        values = export.result()
+        if not values:
+            return True
+        if all((val in self.valid_solidity_function or self._is_constant_var(val) for val in values)):
+            return True
+        return False
+
+
+
     def detect(self):
         """ Detect state variables that could be const
         """
@@ -42,7 +76,7 @@ def detect(self):
         all_variables_written = set([item for sublist in all_variables_written for item in sublist])
 
         constable_variables = [v for v in all_non_constant_elementary_variables
-                               if not v in all_variables_written]
+                               if (not v in all_variables_written) and self._constant_initial_expression(v)]
         # Order for deterministic results
         constable_variables = sorted(constable_variables, key=lambda x: x.canonical_name)
         for v in constable_variables:

tests/const_state_variables.sol

@@ -35,3 +35,18 @@ contract B is A {
         }
     }
 }
+
+contract MyConc{
+
+    uint constant A = 1;
+    bytes32 should_be_constant = sha256('abc');
+    uint should_be_constant_2 = A + 1;
+    address not_constant = msg.sender;
+    uint not_constant_2 = getNumber();
+    uint not_constant_3 = 10 + block.number;
+
+    function getNumber() public returns(uint){
+        return block.number;
+    }
+
+}

tests/expected_json/const_state_variables.constable-states.json

@@ -1 +1 @@
-[{"check": "constable-states", "impact": "Informational", "confidence": "High", "description": "A.myFriendsAddress should be constant (tests/const_state_variables.sol#7)\nA.test should be constant (tests/const_state_variables.sol#10)\nA.text2 should be constant (tests/const_state_variables.sol#14)\nB.mySistersAddress should be constant (tests/const_state_variables.sol#26)\n", "elements": [{"type": "variable", "name": "myFriendsAddress", "source_mapping": {"start": 132, "length": 76, "filename": "tests/const_state_variables.sol", "lines": [7]}}, {"type": "variable", "name": "mySistersAddress", "source_mapping": {"start": 496, "length": 76, "filename": "tests/const_state_variables.sol", "lines": [26]}}, {"type": "variable", "name": "test", "source_mapping": {"start": 237, "length": 20, "filename": "tests/const_state_variables.sol", "lines": [10]}}, {"type": "variable", "name": "text2", "source_mapping": {"start": 333, "length": 20, "filename": "tests/const_state_variables.sol", "lines": [14]}}]}]
+[{"check": "constable-states", "impact": "Informational", "confidence": "High", "description": "A.myFriendsAddress should be constant (tests/const_state_variables.sol#7)\nA.test should be constant (tests/const_state_variables.sol#10)\nA.text2 should be constant (tests/const_state_variables.sol#14)\nB.mySistersAddress should be constant (tests/const_state_variables.sol#26)\nMyConc.should_be_constant should be constant (tests/const_state_variables.sol#42)\nMyConc.should_be_constant_2 should be constant (tests/const_state_variables.sol#43)\n", "elements": [{"type": "variable", "name": "myFriendsAddress", "source_mapping": {"start": 132, "length": 76, "filename": "tests/const_state_variables.sol", "lines": [7]}}, {"type": "variable", "name": "mySistersAddress", "source_mapping": {"start": 496, "length": 76, "filename": "tests/const_state_variables.sol", "lines": [26]}}, {"type": "variable", "name": "should_be_constant", "source_mapping": {"start": 793, "length": 42, "filename": "tests/const_state_variables.sol", "lines": [42]}}, {"type": "variable", "name": "should_be_constant_2", "source_mapping": {"start": 841, "length": 33, "filename": "tests/const_state_variables.sol", "lines": [43]}}, {"type": "variable", "name": "test", "source_mapping": {"start": 237, "length": 20, "filename": "tests/const_state_variables.sol", "lines": [10]}}, {"type": "variable", "name": "text2", "source_mapping": {"start": 333, "length": 20, "filename": "tests/const_state_variables.sol", "lines": [14]}}]}]