Merge branch 'const-func/split' of https://github.com/uivlis/slither into uivlis-const-func/split

scripts/tests_generate_expected_json_4.sh

@@ -46,7 +46,8 @@ generate_expected_json tests/reentrancy.sol "reentrancy-unlimited-gas"
 #generate_expected_json tests/naming_convention.sol "naming-convention"
 #generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
 #generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
-#generate_expected_json tests/constant.sol "constant-function"
+#generate_expected_json tests/constant.sol "constant-function-asm"
+#generate_expected_json tests/constant.sol "constant-function-state"
 #generate_expected_json tests/unused_return.sol "unused-return"
 #generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
 #generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"

scripts/tests_generate_expected_json_5.sh

@@ -32,7 +32,8 @@ generate_expected_json(){
 #generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send"
 #generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly"
 #generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly"
-#generate_expected_json tests/constant-0.5.1.sol "constant-function"
+#generate_expected_json tests/constant-0.5.1.sol "constant-function-asm"
+#generate_expected_json tests/constant-0.5.1.sol "constant-function-state"
 #generate_expected_json tests/incorrect_equality.sol "incorrect-equality"
 #generate_expected_json tests/too_many_digits.sol "too-many-digits"
 #generate_expected_json tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel"

scripts/travis_test_4.sh

@@ -95,7 +95,8 @@ test_slither tests/naming_convention.sol "naming-convention"
 #test_slither tests/complex_func.sol "complex-function"
 test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
 test_slither tests/uninitialized_local_variable.sol "uninitialized-local"
-test_slither tests/constant.sol "constant-function"
+test_slither tests/constant.sol "constant-function-asm"
+test_slither tests/constant.sol "constant-function-state"
 test_slither tests/unused_return.sol "unused-return"
 test_slither tests/shadowing_abstract.sol "shadowing-abstract"
 test_slither tests/shadowing_state_variable.sol "shadowing-state"

scripts/travis_test_5.sh

@@ -91,7 +91,8 @@ test_slither tests/external_function_2.sol "external-function"
 test_slither tests/naming_convention.sol "naming-convention"
 #test_slither tests/complex_func.sol "complex-function"
 test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
-test_slither tests/constant-0.5.1.sol "constant-function"
+test_slither tests/constant-0.5.1.sol "constant-function-asm"
+test_slither tests/constant-0.5.1.sol "constant-function-state"
 test_slither tests/unused_return.sol "unused-return"
 test_slither tests/timestamp.sol "timestamp"
 test_slither tests/incorrect_equality.sol "incorrect-equality"

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-asm'  # 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,12 +6,12 @@ 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
     """
 
-    ARGUMENT = 'constant-function'  # run the detector with slither.py --ARGUMENT
+    ARGUMENT = 'constant-function-state'  # run the detector with slither.py --ARGUMENT
     HELP = 'Constant functions changing the state'  # help information
     IMPACT = DetectorClassification.MEDIUM
     CONFIDENCE = DetectorClassification.MEDIUM
@@ -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:
+        if self.slither._solc_version < "0.5.0":
-            for f in c.functions:
+            for c in self.contracts:
-                if f.contract_declarer != c:
+                for f in c.functions:
-                    continue
+                    if f.contract_declarer != c:
-                if f.view or f.pure:
+                        continue
-                    if f.contains_assembly:
+                    if f.view or f.pure:
-                        attr = 'view' if f.view else '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']
+                            info = [f, f' is declared {attr} but changes state variables:\n']
-                        res = self.generate_result(info, {'contains_assembly': True})
 
-                        results.append(res)
+                            for variable_written in variables_written:
+                                info += ['\t- ', variable_written, '\n']
 
-                    variables_written = f.all_state_variables_written()
+                            res = self.generate_result(info, {'contains_assembly': False})
-                    if variables_written:
-                        attr = 'view' if f.view else 'pure'
 
-                        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']
-
-                        res = self.generate_result(info, {'contains_assembly': False})
-
-                        results.append(res)
 
         return results
 

slither/tools/slither_format/__main__.py

@@ -16,7 +16,8 @@ available_detectors = ["unused-state",
                        "naming-convention",
                        "external-function",
                        "constable-states",
-                       "constant-function"]
+                       "constant-function-asm",
+                       "constatnt-function-state"]
 
 detectors_to_run = []
 

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/constant-0.5.1.sol

@@ -1,3 +1,5 @@
+pragma solidity 0.5.1;
+
 contract Constant {
    
     uint a;

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

@@ -0,0 +1,5 @@
+{
+  "success": true,
+  "error": null,
+  "results": {}
+}

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

@@ -0,0 +1,5 @@
+{
+  "success": true,
+  "error": null,
+  "results": {}
+}

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

@@ -1,4 +0,0 @@
-
-Constant.test_assembly_bug() (tests/constant-0.5.1.sol#15-17) is declared view but contains assembly code
-Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state
-tests/constant-0.5.1.sol analyzed (1 contracts with 1 detectors), 1 result(s) found

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

@@ -9,17 +9,17 @@
             "type": "function",
             "name": "test_assembly_bug",
             "source_mapping": {
-              "start": 185,
+              "start": 324,
               "length": 66,
-              "filename_used": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
+              "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
-              "filename_relative": "tests/constant-0.5.1.sol",
+              "filename_relative": "tests/constant.sol",
-              "filename_absolute": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
+              "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
-              "filename_short": "tests/constant-0.5.1.sol",
+              "filename_short": "tests/constant.sol",
               "is_dependency": false,
               "lines": [
-                15,
+                22,
-                16,
+                23,
-                17
+                24
               ],
               "starting_column": 5,
               "ending_column": 6
@@ -30,11 +30,11 @@
                 "name": "Constant",
                 "source_mapping": {
                   "start": 0,
-                  "length": 253,
+                  "length": 392,
-                  "filename_used": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
+                  "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
-                  "filename_relative": "tests/constant-0.5.1.sol",
+                  "filename_relative": "tests/constant.sol",
-                  "filename_absolute": "/home/travis/build/crytic/slither/tests/constant-0.5.1.sol",
+                  "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
-                  "filename_short": "tests/constant-0.5.1.sol",
+                  "filename_short": "tests/constant.sol",
                   "is_dependency": false,
                   "lines": [
                     1,
@@ -54,7 +54,14 @@
                     15,
                     16,
                     17,
-                    18
+                    18,
+                    19,
+                    20,
+                    21,
+                    22,
+                    23,
+                    24,
+                    25
                   ],
                   "starting_column": 1,
                   "ending_column": 2
@@ -64,13 +71,13 @@
             }
           }
         ],
-        "description": "Constant.test_assembly_bug() (tests/constant-0.5.1.sol#15-17) is declared view but contains assembly code\n",
+        "description": "Constant.test_assembly_bug() (tests/constant.sol#22-24) 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",
+        "markdown": "[Constant.test_assembly_bug()](tests/constant.sol#L22-L24) is declared view but contains assembly code\n",
         "id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23",
         "additional_fields": {
           "contains_assembly": true
         },
-        "check": "constant-function",
+        "check": "constant-function-asm",
         "impact": "Medium",
         "confidence": "Medium"
       }

tests/expected_json/constant.constant-function-asm.txt

@@ -0,0 +1,4 @@
+
+Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code
+Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state
+tests/constant.sol analyzed (1 contracts with 1 detectors), 3 result(s) found

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

@@ -139,7 +139,7 @@
         "additional_fields": {
           "contains_assembly": false
         },
-        "check": "constant-function",
+        "check": "constant-function-state",
         "impact": "Medium",
         "confidence": "Medium"
       },
@@ -279,85 +279,7 @@
         "additional_fields": {
           "contains_assembly": false
         },
-        "check": "constant-function",
+        "check": "constant-function-state",
-        "impact": "Medium",
-        "confidence": "Medium"
-      },
-      {
-        "elements": [
-          {
-            "type": "function",
-            "name": "test_assembly_bug",
-            "source_mapping": {
-              "start": 324,
-              "length": 66,
-              "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
-              "filename_relative": "tests/constant.sol",
-              "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
-              "filename_short": "tests/constant.sol",
-              "is_dependency": false,
-              "lines": [
-                22,
-                23,
-                24
-              ],
-              "starting_column": 5,
-              "ending_column": 6
-            },
-            "type_specific_fields": {
-              "parent": {
-                "type": "contract",
-                "name": "Constant",
-                "source_mapping": {
-                  "start": 0,
-                  "length": 392,
-                  "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol",
-                  "filename_relative": "tests/constant.sol",
-                  "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol",
-                  "filename_short": "tests/constant.sol",
-                  "is_dependency": false,
-                  "lines": [
-                    1,
-                    2,
-                    3,
-                    4,
-                    5,
-                    6,
-                    7,
-                    8,
-                    9,
-                    10,
-                    11,
-                    12,
-                    13,
-                    14,
-                    15,
-                    16,
-                    17,
-                    18,
-                    19,
-                    20,
-                    21,
-                    22,
-                    23,
-                    24,
-                    25
-                  ],
-                  "starting_column": 1,
-                  "ending_column": 2
-                }
-              },
-              "signature": "test_assembly_bug()"
-            }
-          }
-        ],
-        "description": "Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code\n",
-        "markdown": "[Constant.test_assembly_bug()](tests/constant.sol#L22-L24) is declared view but contains assembly code\n",
-        "id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23",
-        "additional_fields": {
-          "contains_assembly": true
-        },
-        "check": "constant-function",
         "impact": "Medium",
         "confidence": "Medium"
       }

tests/expected_json/constant.constant-function-state.txt

@@ -3,6 +3,5 @@ Constant.test_view_bug() (tests/constant.sol#5-7) is declared view but changes s
 	- Constant.a (tests/constant.sol#3)
 Constant.test_constant_bug() (tests/constant.sol#9-11) is declared view but changes state variables:
 	- Constant.a (tests/constant.sol#3)
-Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code
 Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state
 tests/constant.sol analyzed (1 contracts with 1 detectors), 3 result(s) found