From fc4ac0c988ef098cf06b8b934e70b4b39abd8a12 Mon Sep 17 00:00:00 2001 From: Josselin Date: Wed, 9 Jan 2019 14:19:05 +0000 Subject: [PATCH] Improve constable-states result (remove FP) --- .../possible_const_state_variables.py | 36 ++++++++++++++++++- tests/const_state_variables.sol | 15 ++++++++ ...onst_state_variables.constable-states.json | 2 +- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/slither/detectors/variables/possible_const_state_variables.py b/slither/detectors/variables/possible_const_state_variables.py index 65f0dda22..49214f437 100644 --- a/slither/detectors/variables/possible_const_state_variables.py +++ b/slither/detectors/variables/possible_const_state_variables.py @@ -4,6 +4,9 @@ Module detecting state variables that could be declared as constant 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 @@ class ConstCandidateStateVars(AbstractDetector): 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: diff --git a/tests/const_state_variables.sol b/tests/const_state_variables.sol index 94c940dd6..aed05d97f 100644 --- a/tests/const_state_variables.sol +++ b/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; + } + +} diff --git a/tests/expected_json/const_state_variables.constable-states.json b/tests/expected_json/const_state_variables.constable-states.json index 39a218402..575fdd28c 100644 --- a/tests/expected_json/const_state_variables.constable-states.json +++ b/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]}}]}] \ No newline at end of file +[{"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]}}]}] \ No newline at end of file