external function detector:

- Improve function_parameters_written - update travis tests
5 years ago · b2c7237cce
parent 9f49a95325
commit b2c7237cce
4 changed files with 93 additions and 38 deletions
--- a/scripts/tests_generate_expected_json_4.sh
+++ b/scripts/tests_generate_expected_json_4.sh
@ -21,38 +21,38 @@ generate_expected_json(){
 }


-generate_expected_json tests/deprecated_calls.sol "deprecated-standards"
-generate_expected_json tests/erc20_indexed.sol "erc20-indexed"
-generate_expected_json tests/incorrect_erc20_interface.sol "erc20-interface"
-generate_expected_json tests/incorrect_erc721_interface.sol "erc721-interface"
-generate_expected_json tests/uninitialized.sol "uninitialized-state"
-generate_expected_json tests/backdoor.sol "backdoor"
-generate_expected_json tests/backdoor.sol "suicidal"
-generate_expected_json tests/pragma.0.4.24.sol "pragma"
-generate_expected_json tests/old_solc.sol.json "solc-version"
-generate_expected_json tests/reentrancy.sol "reentrancy-eth"
-generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
-generate_expected_json tests/tx_origin.sol "tx-origin"
-generate_expected_json tests/unused_state.sol "unused-state"
-generate_expected_json tests/locked_ether.sol "locked-ether"
-generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
-generate_expected_json tests/inline_assembly_contract.sol "assembly"
-generate_expected_json tests/inline_assembly_library.sol "assembly"
-generate_expected_json tests/low_level_calls.sol "low-level-calls"
-generate_expected_json tests/const_state_variables.sol "constable-states"
+#generate_expected_json tests/deprecated_calls.sol "deprecated-standards"
+#generate_expected_json tests/erc20_indexed.sol "erc20-indexed"
+#generate_expected_json tests/incorrect_erc20_interface.sol "erc20-interface"
+#generate_expected_json tests/incorrect_erc721_interface.sol "erc721-interface"
+#generate_expected_json tests/uninitialized.sol "uninitialized-state"
+#generate_expected_json tests/backdoor.sol "backdoor"
+#generate_expected_json tests/backdoor.sol "suicidal"
+#generate_expected_json tests/pragma.0.4.24.sol "pragma"
+#generate_expected_json tests/old_solc.sol.json "solc-version"
+#generate_expected_json tests/reentrancy.sol "reentrancy-eth"
+#generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
+#generate_expected_json tests/tx_origin.sol "tx-origin"
+#generate_expected_json tests/unused_state.sol "unused-state"
+#generate_expected_json tests/locked_ether.sol "locked-ether"
+#generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
+#generate_expected_json tests/inline_assembly_contract.sol "assembly"
+#generate_expected_json tests/inline_assembly_library.sol "assembly"
+#generate_expected_json tests/low_level_calls.sol "low-level-calls"
+#generate_expected_json tests/const_state_variables.sol "constable-states"
 generate_expected_json tests/external_function.sol "external-function"
 generate_expected_json tests/external_function_2.sol "external-function"
-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/unused_return.sol "unused-return"
-generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
-generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
-generate_expected_json tests/timestamp.sol "timestamp"
-generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
-generate_expected_json tests/shadowing_builtin_symbols.sol "shadowing-builtin"
-generate_expected_json tests/shadowing_local_variable.sol "shadowing-local"
-generate_expected_json tests/solc_version_incorrect.sol "solc-version"
-generate_expected_json tests/right_to_left_override.sol "rtlo"
-generate_expected_json tests/unchecked_lowlevel.sol "unchecked-lowlevel"
+#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/unused_return.sol "unused-return"
+#generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
+#generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
+#generate_expected_json tests/timestamp.sol "timestamp"
+#generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
+#generate_expected_json tests/shadowing_builtin_symbols.sol "shadowing-builtin"
+#generate_expected_json tests/shadowing_local_variable.sol "shadowing-local"
+#generate_expected_json tests/solc_version_incorrect.sol "solc-version"
+#generate_expected_json tests/right_to_left_override.sol "rtlo"
+#generate_expected_json tests/unchecked_lowlevel.sol "unchecked-lowlevel"
--- a/slither/detectors/functions/external_function.py
+++ b/slither/detectors/functions/external_function.py
@ -98,10 +98,7 @@ class ExternalFunction(AbstractDetector):

    @staticmethod
    def function_parameters_written(function):
-        for node in function.nodes:
-            if any (var.name == parameter.name for var in node.local_variables_written for parameter in function.parameters):
-                return True
-        return False
+        return any(p in function.variables_written for p in function.parameters)

    def _detect(self):
        results = []
--- a/tests/expected_json/external_function.external-function.json
+++ b/tests/expected_json/external_function.external-function.json
@ -258,6 +258,63 @@
            }
          }
        ]
+      },
+      {
+        "check": "external-function",
+        "impact": "Informational",
+        "confidence": "High",
+        "description": "FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76) should be declared external\n",
+        "elements": [
+          {
+            "type": "function",
+            "name": "parameter_read_ok_for_external",
+            "source_mapping": {
+              "start": 1420,
+              "length": 81,
+              "filename_used": "/home/travis/build/crytic/slither/tests/external_function.sol",
+              "filename_relative": "tests/external_function.sol",
+              "filename_absolute": "/home/travis/build/crytic/slither/tests/external_function.sol",
+              "filename_short": "tests/external_function.sol",
+              "lines": [
+                74,
+                75,
+                76
+              ],
+              "starting_column": 3,
+              "ending_column": 4
+            },
+            "type_specific_fields": {
+              "parent": {
+                "type": "contract",
+                "name": "FunctionParameterWrite",
+                "source_mapping": {
+                  "start": 1381,
+                  "length": 234,
+                  "filename_used": "/home/travis/build/crytic/slither/tests/external_function.sol",
+                  "filename_relative": "tests/external_function.sol",
+                  "filename_absolute": "/home/travis/build/crytic/slither/tests/external_function.sol",
+                  "filename_short": "tests/external_function.sol",
+                  "lines": [
+                    72,
+                    73,
+                    74,
+                    75,
+                    76,
+                    77,
+                    78,
+                    79,
+                    80,
+                    81,
+                    82
+                  ],
+                  "starting_column": 1,
+                  "ending_column": 2
+                }
+              },
+              "signature": "parameter_read_ok_for_external(uint256)"
+            }
+          }
+        ]
      }
    ]
  }
--- a/tests/expected_json/external_function.external-function.txt
+++ b/tests/expected_json/external_function.external-function.txt
@ -3,5 +3,6 @@ ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-1
 ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19) should be declared external
 ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23) should be declared external
 ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39) should be declared external
+FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76) should be declared external
 Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external[0m
-INFO:Slither:tests/external_function.sol analyzed (5 contracts), 4 result(s) found
+INFO:Slither:tests/external_function.sol analyzed (6 contracts), 5 result(s) found