From b9d5e172451228eda76f1a4c1abfa89d1a28deca Mon Sep 17 00:00:00 2001 From: Josselin Date: Wed, 9 Jan 2019 08:19:42 +0000 Subject: [PATCH] Simplify constable detector (+ group all the result in one json) --- .../possible_const_state_variables.py | 73 +++++++------------ ...onst_state_variables.constable-states.json | 2 +- 2 files changed, 26 insertions(+), 49 deletions(-) diff --git a/slither/detectors/variables/possible_const_state_variables.py b/slither/detectors/variables/possible_const_state_variables.py index 6437c91a7..65f0dda22 100644 --- a/slither/detectors/variables/possible_const_state_variables.py +++ b/slither/detectors/variables/possible_const_state_variables.py @@ -2,13 +2,8 @@ Module detecting state variables that could be declared as constant """ -from collections import defaultdict from slither.core.solidity_types.elementary_type import ElementaryType from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification -from slither.slithir.operations import OperationWithLValue -from slither.core.variables.state_variable import StateVariable -from slither.core.expressions.literal import Literal - class ConstCandidateStateVars(AbstractDetector): """ @@ -26,56 +21,38 @@ class ConstCandidateStateVars(AbstractDetector): WIKI = 'https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#state-variables-that-could-be-declared-constant' @staticmethod - def lvalues_of_operations_with_lvalue(contract): - ret = [] - for f in contract.all_functions_called + contract.modifiers: - for n in f.nodes: - for ir in n.irs: - if isinstance(ir, OperationWithLValue) and isinstance(ir.lvalue, StateVariable): - ret.append(ir.lvalue) - return ret - - @staticmethod - def non_const_state_variables(contract): - return [variable for variable in contract.state_variables - if not variable.is_constant and type(variable.expression) == Literal] - - def detect_const_candidates(self, contract): - const_candidates = [] - non_const_state_vars = self.non_const_state_variables(contract) - lvalues_of_operations = self.lvalues_of_operations_with_lvalue(contract) - for non_const in non_const_state_vars: - if non_const not in lvalues_of_operations \ - and non_const not in const_candidates \ - and isinstance(non_const.type, ElementaryType): - const_candidates.append(non_const) - - return const_candidates + def _valid_candidate(v): + return isinstance(v.type, ElementaryType) and not v.is_constant def detect(self): """ Detect state variables that could be const """ results = [] all_info = '' - for c in self.slither.contracts_derived: - const_candidates = self.detect_const_candidates(c) - if const_candidates: - variables_by_contract = defaultdict(list) - - for state_var in const_candidates: - variables_by_contract[state_var.contract.name].append(state_var) - - for contract, variables in variables_by_contract.items(): - info = '' - for v in variables: - info += "{}.{} should be constant ({})\n".format(contract, - v.name, - v.source_mapping_str) - all_info += info - json = self.generate_json_result(info) - self.add_variables_to_json(variables, json) - results.append(json) + all_variables = [c.state_variables for c in self.slither.contracts] + all_variables = set([item for sublist in all_variables for item in sublist]) + all_non_constant_elementary_variables = set([v for v in all_variables + if self._valid_candidate(v)]) + + all_functions = [c.all_functions_called for c in self.slither.contracts] + all_functions = list(set([item for sublist in all_functions for item in sublist])) + + all_variables_written = [f.state_variables_written for f in all_functions] + 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] + # Order for deterministic results + constable_variables = sorted(constable_variables, key=lambda x: x.canonical_name) + for v in constable_variables: + info = "{}.{} should be constant ({})\n".format(v.contract.name, + v.name, + v.source_mapping_str) + all_info += info if all_info != '': + json = self.generate_json_result(all_info) + self.add_variables_to_json(constable_variables, json) + results.append(json) self.log(all_info) return results diff --git a/tests/expected_json/const_state_variables.constable-states.json b/tests/expected_json/const_state_variables.constable-states.json index d44c1fa72..39a218402 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)\n", "elements": [{"type": "variable", "name": "myFriendsAddress", "source_mapping": {"start": 132, "length": 76, "filename": "tests/const_state_variables.sol", "lines": [7]}}, {"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": "B.mySistersAddress should be constant (tests/const_state_variables.sol#26)\n", "elements": [{"type": "variable", "name": "mySistersAddress", "source_mapping": {"start": 496, "length": 76, "filename": "tests/const_state_variables.sol", "lines": [26]}}]}] \ 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)\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