Merge pull request #1455 from crytic/detect/add-immutable-opti

add immutable-states detector

slither/detectors/all_detectors.py

@@ -19,7 +19,8 @@
 from .reentrancy.reentrancy_no_gas import ReentrancyNoGas
 from .reentrancy.reentrancy_events import ReentrancyEvent
 from .variables.unused_state_variables import UnusedStateVars
-from .variables.possible_const_state_variables import ConstCandidateStateVars
+from .variables.could_be_constant import CouldBeConstant
+from .variables.could_be_immutable import CouldBeImmutable
 from .statements.tx_origin import TxOrigin
 from .statements.assembly import Assembly
 from .operations.low_level_calls import LowLevelCalls

slither/detectors/variables/could_be_constant.py

@@ -0,0 +1,45 @@
+from typing import List, Dict
+from slither.utils.output import Output
+from slither.core.compilation_unit import SlitherCompilationUnit
+from slither.formatters.variables.unchanged_state_variables import custom_format
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from .unchanged_state_variables import UnchangedStateVariables
+
+
+class CouldBeConstant(AbstractDetector):
+    """
+    State variables that could be declared as constant.
+    Not all types for constants are implemented in Solidity as of 0.4.25.
+    The only supported types are value types and strings (ElementaryType).
+    Reference: https://solidity.readthedocs.io/en/latest/contracts.html#constant-state-variables
+    """
+
+    ARGUMENT = "constable-states"
+    HELP = "State variables that could be declared constant"
+    IMPACT = DetectorClassification.OPTIMIZATION
+    CONFIDENCE = DetectorClassification.HIGH
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant"
+
+    WIKI_TITLE = "State variables that could be declared constant"
+    WIKI_DESCRIPTION = "State variables that are not updated following deployment should be declared constant to save gas."
+    WIKI_RECOMMENDATION = "Add the `constant` attribute to state variables that never change."
+
+    def _detect(self) -> List[Output]:
+        """Detect state variables that could be constant"""
+        results = {}
+
+        unchanged_state_variables = UnchangedStateVariables(self.compilation_unit)
+        unchanged_state_variables.detect()
+
+        for variable in unchanged_state_variables.constant_candidates:
+            results[variable.canonical_name] = self.generate_result(
+                [variable, " should be constant \n"]
+            )
+
+        # Order by canonical name for deterministic results
+        return [results[k] for k in sorted(results)]
+
+    @staticmethod
+    def _format(compilation_unit: SlitherCompilationUnit, result: Dict) -> None:
+        custom_format(compilation_unit, result, "constant")

slither/detectors/variables/could_be_immutable.py

@@ -0,0 +1,44 @@
+from typing import List, Dict
+from slither.utils.output import Output
+from slither.core.compilation_unit import SlitherCompilationUnit
+from slither.formatters.variables.unchanged_state_variables import custom_format
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from .unchanged_state_variables import UnchangedStateVariables
+
+
+class CouldBeImmutable(AbstractDetector):
+    """
+    State variables that could be declared immutable.
+    # Immutable attribute available in Solidity 0.6.5 and above
+    # https://blog.soliditylang.org/2020/04/06/solidity-0.6.5-release-announcement/
+    """
+
+    # VULNERABLE_SOLC_VERSIONS =
+    ARGUMENT = "immutable-states"
+    HELP = "State variables that could be declared immutable"
+    IMPACT = DetectorClassification.OPTIMIZATION
+    CONFIDENCE = DetectorClassification.HIGH
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable"
+
+    WIKI_TITLE = "State variables that could be declared immutable"
+    WIKI_DESCRIPTION = "State variables that are not updated following deployment should be declared immutable to save gas."
+    WIKI_RECOMMENDATION = "Add the `immutable` attribute to state variables that never change or are set only in the constructor."
+
+    def _detect(self) -> List[Output]:
+        """Detect state variables that could be immutable"""
+        results = {}
+        unchanged_state_variables = UnchangedStateVariables(self.compilation_unit)
+        unchanged_state_variables.detect()
+
+        for variable in unchanged_state_variables.immutable_candidates:
+            results[variable.canonical_name] = self.generate_result(
+                [variable, " should be immutable \n"]
+            )
+
+        # Order by canonical name for deterministic results
+        return [results[k] for k in sorted(results)]
+
+    @staticmethod
+    def _format(compilation_unit: SlitherCompilationUnit, result: Dict) -> None:
+        custom_format(compilation_unit, result, "immutable")

slither/detectors/variables/unchanged_state_variables.py

@@ -0,0 +1,125 @@
+"""
+Module detecting state variables that could be declared as constant
+"""
+from typing import Set, List
+from packaging import version
+from slither.core.compilation_unit import SlitherCompilationUnit
+from slither.core.solidity_types.elementary_type import ElementaryType
+from slither.core.solidity_types.user_defined_type import UserDefinedType
+from slither.core.variables.variable import Variable
+
+from slither.visitors.expression.export_values import ExportValues
+from slither.core.declarations import Contract, Function
+from slither.core.declarations.solidity_variables import SolidityFunction
+from slither.core.variables.state_variable import StateVariable
+from slither.core.expressions import CallExpression, NewContract
+
+
+def _is_valid_type(v: StateVariable) -> bool:
+    t = v.type
+    if isinstance(t, ElementaryType):
+        return True
+    if isinstance(t, UserDefinedType) and isinstance(t.type, Contract):
+        return True
+    return False
+
+
+def _valid_candidate(v: StateVariable) -> bool:
+    return _is_valid_type(v) and not (v.is_constant or v.is_immutable)
+
+
+def _is_constant_var(v: Variable) -> bool:
+    if isinstance(v, StateVariable):
+        return v.is_constant
+    return False
+
+
+# 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)"),
+]
+
+
+def _constant_initial_expression(v: Variable) -> bool:
+    if not v.expression:
+        return True
+
+    # B b = new B(); b cannot be constant, so filter out and recommend it be immutable
+    if isinstance(v.expression, CallExpression) and isinstance(v.expression.called, NewContract):
+        return False
+
+    export = ExportValues(v.expression)
+    values = export.result()
+    if not values:
+        return True
+
+    return all((val in valid_solidity_function or _is_constant_var(val) for val in values))
+
+
+class UnchangedStateVariables:
+    """
+    Find state variables that could be declared as constant or immutable (not written after deployment).
+    """
+
+    def __init__(self, compilation_unit: SlitherCompilationUnit):
+        self.compilation_unit = compilation_unit
+        self._constant_candidates: List[StateVariable] = []
+        self._immutable_candidates: List[StateVariable] = []
+
+    @property
+    def immutable_candidates(self) -> List[StateVariable]:
+        """Return the immutable candidates"""
+        return self._immutable_candidates
+
+    @property
+    def constant_candidates(self) -> List[StateVariable]:
+        """Return the constant candidates"""
+        return self._constant_candidates
+
+    def detect(self):
+        """Detect state variables that could be constant or immutable"""
+        for c in self.compilation_unit.contracts_derived:
+            variables = []
+            functions = []
+
+            variables.append(c.state_variables)
+            functions.append(c.all_functions_called)
+
+            valid_candidates: Set[StateVariable] = {
+                item for sublist in variables for item in sublist if _valid_candidate(item)
+            }
+
+            all_functions: List[Function] = list(
+                {item1 for sublist in functions for item1 in sublist if isinstance(item1, Function)}
+            )
+
+            variables_written = []
+            constructor_variables_written = []
+            variables_initialized = []
+            for f in all_functions:
+                if f.is_constructor_variables:
+                    variables_initialized.extend(f.state_variables_written)
+                elif f.is_constructor:
+                    constructor_variables_written.extend(f.state_variables_written)
+                else:
+                    variables_written.extend(f.state_variables_written)
+
+            for v in valid_candidates:
+                if v not in variables_written:
+                    if _constant_initial_expression(v) and v not in constructor_variables_written:
+                        self.constant_candidates.append(v)
+
+                    elif (
+                        v in constructor_variables_written or v in variables_initialized
+                    ) and version.parse(self.compilation_unit.solc_version) >= version.parse(
+                        "0.6.5"
+                    ):
+                        self.immutable_candidates.append(v)

slither/formatters/variables/possible_const_state_variables.py → slither/formatters/variables/unchanged_state_variables.py

@@ -5,7 +5,7 @@
 from slither.formatters.utils.patches import create_patch
 
 
-def custom_format(compilation_unit: SlitherCompilationUnit, result):
+def custom_format(compilation_unit: SlitherCompilationUnit, result, attribute: str) -> None:
     elements = result["elements"]
     for element in elements:
 
@@ -15,14 +15,14 @@ def custom_format(compilation_unit: SlitherCompilationUnit, result):
         contract = scope.get_contract_from_name(contract_name)
         var = contract.get_state_variable_from_name(element["name"])
         if not var.expression:
-            raise FormatImpossible(f"{var.name} is uninitialized and cannot become constant.")
+            raise FormatImpossible(f"{var.name} is uninitialized and cannot become {attribute}.")
 
         _patch(
             compilation_unit,
             result,
             element["source_mapping"]["filename_absolute"],
             element["name"],
-            "constant " + element["name"],
+            f"{attribute} " + element["name"],
             element["source_mapping"]["start"],
             element["source_mapping"]["start"] + element["source_mapping"]["length"],
         )

slither/tools/slither_format/slither_format.py

@@ -9,7 +9,8 @@
 from slither.detectors.attributes.constant_pragma import ConstantPragma
 from slither.detectors.naming_convention.naming_convention import NamingConvention
 from slither.detectors.functions.external_function import ExternalFunction
-from slither.detectors.variables.possible_const_state_variables import ConstCandidateStateVars
+from slither.detectors.variables.could_be_constant import CouldBeConstant
+from slither.detectors.variables.could_be_immutable import CouldBeImmutable
 from slither.detectors.attributes.const_functions_asm import ConstantFunctionsAsm
 from slither.detectors.attributes.const_functions_state import ConstantFunctionsState
 from slither.utils.colors import yellow
@@ -23,7 +24,8 @@
     "pragma": ConstantPragma,
     "naming-convention": NamingConvention,
     "external-function": ExternalFunction,
-    "constable-states": ConstCandidateStateVars,
+    "constable-states": CouldBeConstant,
+    "immutable-states": CouldBeImmutable,
     "constant-function-asm": ConstantFunctionsAsm,
     "constant-functions-state": ConstantFunctionsState,
 }