From 8eede9d95fa726d31b4d021d940dfe9db7b1a9c7 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 1 Apr 2022 01:05:34 +0200 Subject: [PATCH] Detect if function has at least one memory arg --- .../detectors/functions/external_function.py | 24 +++++++ ..._function.sol.0.4.25.ExternalFunction.json | 62 ------------------- ..._function.sol.0.5.16.ExternalFunction.json | 62 ------------------- 3 files changed, 24 insertions(+), 124 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 6291d27b3..16ac9d7f1 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -1,3 +1,7 @@ +from array import ArrayType +from slither.core.declarations.structure import Structure +from slither.core.solidity_types.mapping_type import MappingType +from slither.core.solidity_types.user_defined_type import UserDefinedType from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification from slither.slithir.operations import SolidityCall from slither.slithir.operations import InternalCall, InternalDynamicCall @@ -105,6 +109,18 @@ class ExternalFunction(AbstractDetector): def function_parameters_written(function): return any(p in function.variables_written for p in function.parameters) + @staticmethod + def is_reference_type(parameter): + if isinstance(parameter.type, ArrayType): + return True + if isinstance(parameter.type, MappingType): + return True + if isinstance(parameter.type, UserDefinedType) and isinstance(parameter.type.type, Structure): + return True + if str(parameter.type) in ["bytes", "string"]: + return True + return False + def _detect(self): # pylint: disable=too-many-locals,too-many-branches results = [] @@ -135,6 +151,14 @@ class ExternalFunction(AbstractDetector): # Next we'll want to loop through all functions defined directly in this contract. for function in contract.functions_declared: + # If all of the function arguments are non-reference type or calldata, we skip it. + reference_args = [] + for arg in function.parameters: + if self.is_reference_type(arg) and arg.location == 'memory': + reference_args.append(arg) + if len(reference_args) == 0 and len(function.parameters) > 0: + continue + # If the function is a constructor, or is public, we skip it. if function.is_constructor or function.visibility != "public": continue diff --git a/tests/detectors/external-function/0.4.25/external_function.sol.0.4.25.ExternalFunction.json b/tests/detectors/external-function/0.4.25/external_function.sol.0.4.25.ExternalFunction.json index 75e76126d..17885c854 100644 --- a/tests/detectors/external-function/0.4.25/external_function.sol.0.4.25.ExternalFunction.json +++ b/tests/detectors/external-function/0.4.25/external_function.sol.0.4.25.ExternalFunction.json @@ -210,68 +210,6 @@ "impact": "Optimization", "confidence": "High" }, - { - "elements": [ - { - "type": "function", - "name": "parameter_read_ok_for_external", - "source_mapping": { - "start": 1420, - "length": 81, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.4.25/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.4.25/external_function.sol", - "is_dependency": false, - "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": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.4.25/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.4.25/external_function.sol", - "is_dependency": false, - "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)" - } - } - ], - "description": "parameter_read_ok_for_external(uint256) should be declared external:\n\t- FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/detectors/external-function/0.4.25/external_function.sol#74-76)\n", - "markdown": "parameter_read_ok_for_external(uint256) should be declared external:\n\t- [FunctionParameterWrite.parameter_read_ok_for_external(uint256)](tests/detectors/external-function/0.4.25/external_function.sol#L74-L76)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function.sol#L74-L76", - "id": "3a0a42d128eff9fb04d8f7605bf2d6f7574c2cbbdffa2dcabbae66d7568ecc59", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, { "elements": [ { diff --git a/tests/detectors/external-function/0.5.16/external_function.sol.0.5.16.ExternalFunction.json b/tests/detectors/external-function/0.5.16/external_function.sol.0.5.16.ExternalFunction.json index 4d947de06..3a0514955 100644 --- a/tests/detectors/external-function/0.5.16/external_function.sol.0.5.16.ExternalFunction.json +++ b/tests/detectors/external-function/0.5.16/external_function.sol.0.5.16.ExternalFunction.json @@ -210,68 +210,6 @@ "impact": "Optimization", "confidence": "High" }, - { - "elements": [ - { - "type": "function", - "name": "parameter_read_ok_for_external", - "source_mapping": { - "start": 1420, - "length": 81, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.5.16/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.5.16/external_function.sol", - "is_dependency": false, - "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": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.5.16/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.5.16/external_function.sol", - "is_dependency": false, - "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)" - } - } - ], - "description": "parameter_read_ok_for_external(uint256) should be declared external:\n\t- FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/detectors/external-function/0.5.16/external_function.sol#74-76)\n", - "markdown": "parameter_read_ok_for_external(uint256) should be declared external:\n\t- [FunctionParameterWrite.parameter_read_ok_for_external(uint256)](tests/detectors/external-function/0.5.16/external_function.sol#L74-L76)\n", - "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function.sol#L74-L76", - "id": "3a0a42d128eff9fb04d8f7605bf2d6f7574c2cbbdffa2dcabbae66d7568ecc59", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, { "elements": [ {