Split const_functions into state and asm cases. Updated detectors and tests.

slither/detectors/all_detectors.py

@@ -22,7 +22,8 @@ from .operations.unused_return_values import UnusedReturnValues
 from .naming_convention.naming_convention import NamingConvention
 from .functions.external_function import ExternalFunction
 from .statements.controlled_delegatecall import ControlledDelegateCall
-from .attributes.const_functions import ConstantFunctions
+from .attributes.const_functions_asm import ConstantFunctionsAsm
+from .attributes.const_functions_state import ConstantFunctionsState
 from .shadowing.abstract import ShadowingAbstractDetection
 from .shadowing.state import StateShadowing
 from .shadowing.local import LocalShadowing

slither/detectors/attributes/const_functions_asm.py

@@ -0,0 +1,71 @@
+"""
+Module detecting constant functions
+Recursively check the called functions
+"""
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
+from slither.formatters.attributes.const_functions import format
+
+
+class ConstantFunctionsAsm(AbstractDetector):
+    """
+    Constant function detector
+    """
+
+    ARGUMENT = 'constant-function'  # run the detector with slither.py --ARGUMENT
+    HELP = 'Constant functions changing the state'  # help information
+    IMPACT = DetectorClassification.MEDIUM
+    CONFIDENCE = DetectorClassification.MEDIUM
+
+    WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state'
+
+    WIKI_TITLE = 'Constant functions using assembly code'
+    WIKI_DESCRIPTION = '''
+Functions declared as `constant`/`pure`/`view` using assembly code.
+
+`constant`/`pure`/`view` was not enforced prior Solidity 0.5.
+Starting from Solidity 0.5, a call to a `constant`/`pure`/`view` function uses the `STATICCALL` opcode, which reverts in case of state modification.
+
+As a result, a call to an [incorrectly labeled function may trap a contract compiled with Solidity 0.5](https://solidity.readthedocs.io/en/develop/050-breaking-changes.html#interoperability-with-older-contracts).'''
+
+    WIKI_EXPLOIT_SCENARIO = '''
+```solidity
+contract Constant{
+    uint counter;
+    function get() public view returns(uint){
+       counter = counter +1;
+       return counter
+    }
+}
+```
+`Constant` was deployed with Solidity 0.4.25. Bob writes a smart contract interacting with `Constant` in Solidity 0.5.0. 
+All the calls to `get` revert, breaking Bob's smart contract execution.'''
+
+    WIKI_RECOMMENDATION = 'Ensure that the attributes of contracts compiled prior to Solidity 0.5.0 are correct.'
+
+    def _detect(self):
+        """ Detect the constant function using assembly code
+
+        Recursively visit the calls
+        Returns:
+            list: {'vuln', 'filename,'contract','func','#varsWritten'}
+        """
+        results = []
+        if self.slither.solc_version < "0.5.0":
+            for c in self.contracts:
+                for f in c.functions:
+                    if f.contract_declarer != c:
+                        continue
+                    if f.view or f.pure:
+                        if f.contains_assembly:
+                            attr = 'view' if f.view else 'pure'
+
+                            info = [f, f' is declared {attr} but contains assembly code\n']
+                            res = self.generate_result(info, {'contains_assembly': True})
+
+                            results.append(res)
+
+        return results
+
+    @staticmethod
+    def _format(slither, result):
+        format(slither, result)

slither/detectors/attributes/const_functions_state.py

@@ -6,7 +6,7 @@ from slither.detectors.abstract_detector import AbstractDetector, DetectorClassi
 from slither.formatters.attributes.const_functions import format
 
 
-class ConstantFunctions(AbstractDetector):
+class ConstantFunctionsState(AbstractDetector):
     """
     Constant function detector
     """
@@ -20,7 +20,7 @@ class ConstantFunctions(AbstractDetector):
 
     WIKI_TITLE = 'Constant functions changing the state'
     WIKI_DESCRIPTION = '''
-Functions declared as `constant`/`pure`/`view` changing the state or using assembly code.
+Functions declared as `constant`/`pure`/`view` changing the state.
 
 `constant`/`pure`/`view` was not enforced prior Solidity 0.5.
 Starting from Solidity 0.5, a call to a `constant`/`pure`/`view` function uses the `STATICCALL` opcode, which reverts in case of state modification.
@@ -50,31 +50,24 @@ All the calls to `get` revert, breaking Bob's smart contract execution.'''
             list: {'vuln', 'filename,'contract','func','#varsWritten'}
         """
         results = []
-        for c in self.contracts:
-            for f in c.functions:
-                if f.contract_declarer != c:
-                    continue
-                if f.view or f.pure:
-                    if f.contains_assembly:
-                        attr = 'view' if f.view else 'pure'
+        if self.slither.solc_version < "0.5.0":
+            for c in self.contracts:
+                for f in c.functions:
+                    if f.contract_declarer != c:
+                        continue
+                    if f.view or f.pure:
+                        variables_written = f.all_state_variables_written()
+                        if variables_written:
+                            attr = 'view' if f.view else 'pure'
 
-                        info = [f, f' is declared {attr} but contains assembly code\n']
-                        res = self.generate_result(info, {'contains_assembly': True})
+                            info = [f, f' is declared {attr} but changes state variables:\n']
 
-                        results.append(res)
+                            for variable_written in variables_written:
+                                info += ['\t- ', variable_written, '\n']
 
-                    variables_written = f.all_state_variables_written()
-                    if variables_written:
-                        attr = 'view' if f.view else 'pure'
+                            res = self.generate_result(info, {'contains_assembly': False})
 
-                        info = [f, f' is declared {attr} but changes state variables:\n']
-
-                        for variable_written in variables_written:
-                            info += ['\t- ', variable_written, '\n']
-
-                        res = self.generate_result(info, {'contains_assembly': False})
-
-                        results.append(res)
+                            results.append(res)
 
         return results
 

slither/tools/slither_format/slither_format.py

@@ -6,7 +6,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.attributes.const_functions import ConstantFunctions
+from slither.detectors.attributes.const_functions_asm import ConstantFunctionsAsm
+from slither.detectors.attributes.const_functions_state import ConstantFunctionsState
 from slither.utils.colors import yellow
 
 logging.basicConfig(level=logging.INFO)
@@ -19,7 +20,8 @@ all_detectors = {
     'naming-convention': NamingConvention,
     'external-function': ExternalFunction,
     'constable-states' : ConstCandidateStateVars,
-    'constant-function': ConstantFunctions
+    'constant-function-asm': ConstantFunctionsAsm,
+    'constant-functions-state': ConstantFunctionsState
 }
 
 def slither_format(slither, **kwargs):

tests/expected_json/constant-0.5.1.constant-function.json

@@ -1,79 +1,5 @@
 {
   "success": true,
   "error": null,
-  "results": {
-    "detectors": [
-      {
-        "elements": [
-          {
-            "type": "function",
-            "name": "test_assembly_bug",
-            "source_mapping": {
-              "start": 185,
-              "length": 66,
-              "filename_used": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
-              "filename_relative": "tests/constant-0.5.1.sol",
-              "filename_absolute": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
-              "filename_short": "tests/constant-0.5.1.sol",
-              "is_dependency": false,
-              "lines": [
-                15,
-                16,
-                17
-              ],
-              "starting_column": 5,
-              "ending_column": 6
-            },
-            "type_specific_fields": {
-              "parent": {
-                "type": "contract",
-                "name": "Constant",
-                "source_mapping": {
-                  "start": 0,
-                  "length": 253,
-                  "filename_used": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
-                  "filename_relative": "tests/constant-0.5.1.sol",
-                  "filename_absolute": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
-                  "filename_short": "tests/constant-0.5.1.sol",
-                  "is_dependency": false,
-                  "lines": [
-                    1,
-                    2,
-                    3,
-                    4,
-                    5,
-                    6,
-                    7,
-                    8,
-                    9,
-                    10,
-                    11,
-                    12,
-                    13,
-                    14,
-                    15,
-                    16,
-                    17,
-                    18
-                  ],
-                  "starting_column": 1,
-                  "ending_column": 2
-                }
-              },
-              "signature": "test_assembly_bug()"
-            }
-          }
-        ],
-        "description": "Constant.test_assembly_bug() (tests/constant-0.5.1.sol#15-17) is declared view but contains assembly code\n",
-        "markdown": "[Constant.test_assembly_bug()](tests/constant-0.5.1.sol#L15-L17) is declared view but contains assembly code\n",
-        "id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23",
-        "additional_fields": {
-          "contains_assembly": true
-        },
-        "check": "constant-function",
-        "impact": "Medium",
-        "confidence": "Medium"
-      }
-    ]
-  }
+  "results": null
 }

tests/expected_json/constant.constant-function.json

@@ -139,7 +139,7 @@
         "additional_fields": {
           "contains_assembly": false
         },
-        "check": "constant-function",
+        "check": "constant-function-state",
         "impact": "Medium",
         "confidence": "Medium"
       },
@@ -279,7 +279,7 @@
         "additional_fields": {
           "contains_assembly": false
         },
-        "check": "constant-function",
+        "check": "constant-function-state",
         "impact": "Medium",
         "confidence": "Medium"
       },
@@ -357,7 +357,7 @@
         "additional_fields": {
           "contains_assembly": true
         },
-        "check": "constant-function",
+        "check": "constant-function-asm",
         "impact": "Medium",
         "confidence": "Medium"
       }