From 38dcee37447c88ade22207779a97148ce530eaf8 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Mon, 24 Jan 2022 11:27:02 -0300 Subject: [PATCH 01/12] WIP: Filter external visibility recommendation for public functions to solc < 0.6.9 --- slither/detectors/functions/external_function.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index d68b0c31f..3f4c78f0c 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -20,9 +20,9 @@ class ExternalFunction(AbstractDetector): WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external" WIKI_TITLE = "Public function that could be declared external" - WIKI_DESCRIPTION = "`public` functions that are never called by the contract should be declared `external` to save gas." + WIKI_DESCRIPTION = "`public` functions that are never called by the contract should be declared `external` and its immutable parameters should be located in `calldata` to save gas." WIKI_RECOMMENDATION = ( - "Use the `external` attribute for functions never called from the contract." + "Use the `external` attribute for functions never called from the contract, and change the location of immutable parameters to `calldata` to save gas." ) @staticmethod @@ -108,6 +108,10 @@ class ExternalFunction(AbstractDetector): def _detect(self): # pylint: disable=too-many-locals,too-many-branches results = [] + # After solc 0.6.9, calldata arguments are allowed in public functions + if self.compilation_unit.solc_version >= "0.6.9": + return [] + # Create a set to track contracts with dynamic calls. All contracts with dynamic calls could potentially be # calling functions internally, and thus we can't assume any function in such contracts isn't called by them. dynamic_call_contracts = set() From 77a23a94484e6a4881300a5792844d5dd12545ab Mon Sep 17 00:00:00 2001 From: Tadashi Date: Sat, 19 Feb 2022 21:17:10 +0100 Subject: [PATCH 02/12] WIP: Fixed code for detecting solc version --- slither/detectors/functions/external_function.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 3f4c78f0c..c95191ad5 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -109,7 +109,7 @@ class ExternalFunction(AbstractDetector): results = [] # After solc 0.6.9, calldata arguments are allowed in public functions - if self.compilation_unit.solc_version >= "0.6.9": + if self.compilation_unit.solc_version >= "0.7." or self.compilation_unit.solc_version in ["0.6.9","0.6.10","0.6.11"]: return [] # Create a set to track contracts with dynamic calls. All contracts with dynamic calls could potentially be From aceab1243c895c0e3561be482c24125042298c20 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Sat, 19 Feb 2022 22:51:31 +0100 Subject: [PATCH 03/12] Added new tests and updated JSON artifacts --- .../0.4.25/external_function_3.sol | 11 + ...unction_3.sol.0.4.25.ExternalFunction.json | 130 +++++++ .../0.5.16/external_function_3.sol | 7 + ...unction_3.sol.0.5.16.ExternalFunction.json | 63 ++++ ..._function.sol.0.6.11.ExternalFunction.json | 341 +----------------- .../0.6.11/external_function_3.sol | 7 + ...unction_3.sol.0.6.11.ExternalFunction.json | 3 + ...l_function.sol.0.7.6.ExternalFunction.json | 341 +----------------- .../0.7.6/external_function_3.sol | 7 + ...function_3.sol.0.7.6.ExternalFunction.json | 3 + tests/test_detectors.py | 20 + 11 files changed, 253 insertions(+), 680 deletions(-) create mode 100644 tests/detectors/external-function/0.4.25/external_function_3.sol create mode 100644 tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json create mode 100644 tests/detectors/external-function/0.5.16/external_function_3.sol create mode 100644 tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json create mode 100644 tests/detectors/external-function/0.6.11/external_function_3.sol create mode 100644 tests/detectors/external-function/0.6.11/external_function_3.sol.0.6.11.ExternalFunction.json create mode 100644 tests/detectors/external-function/0.7.6/external_function_3.sol create mode 100644 tests/detectors/external-function/0.7.6/external_function_3.sol.0.7.6.ExternalFunction.json diff --git a/tests/detectors/external-function/0.4.25/external_function_3.sol b/tests/detectors/external-function/0.4.25/external_function_3.sol new file mode 100644 index 000000000..2ea376fcd --- /dev/null +++ b/tests/detectors/external-function/0.4.25/external_function_3.sol @@ -0,0 +1,11 @@ +contract Test { + + function test(bytes x) public { + + } + + function test2(bytes memory x) public { + + } + +} \ No newline at end of file diff --git a/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json b/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json new file mode 100644 index 000000000..c7c7ae31e --- /dev/null +++ b/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json @@ -0,0 +1,130 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "test", + "source_mapping": { + "start": 21, + "length": 46, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "is_dependency": false, + "lines": [ + 3, + 4, + 5 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Test", + "source_mapping": { + "start": 0, + "length": 134, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "test(bytes)" + } + } + ], + "description": "test(bytes) should be declared external:\n\t- Test.test(bytes) (tests/detectors/external-function/0.4.25/external_function_3.sol#3-5)\n", + "markdown": "test(bytes) should be declared external:\n\t- [Test.test(bytes)](tests/detectors/external-function/0.4.25/external_function_3.sol#L3-L5)\n", + "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function_3.sol#L3-L5", + "id": "058fd57c313c645eff4b4e53234ca0f73ca3362bd28d3fdc90e0219a32099592", + "check": "external-function", + "impact": "Optimization", + "confidence": "High" + }, + { + "elements": [ + { + "type": "function", + "name": "test2", + "source_mapping": { + "start": 73, + "length": 54, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Test", + "source_mapping": { + "start": 0, + "length": 134, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "test2(bytes)" + } + } + ], + "description": "test2(bytes) should be declared external:\n\t- Test.test2(bytes) (tests/detectors/external-function/0.4.25/external_function_3.sol#7-9)\n", + "markdown": "test2(bytes) should be declared external:\n\t- [Test.test2(bytes)](tests/detectors/external-function/0.4.25/external_function_3.sol#L7-L9)\n", + "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function_3.sol#L7-L9", + "id": "a5b4753f43bb5a2a669ecbf4ce6ab1aaff060454657b16b5ed8cc9c34b521c79", + "check": "external-function", + "impact": "Optimization", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/external-function/0.5.16/external_function_3.sol b/tests/detectors/external-function/0.5.16/external_function_3.sol new file mode 100644 index 000000000..641027603 --- /dev/null +++ b/tests/detectors/external-function/0.5.16/external_function_3.sol @@ -0,0 +1,7 @@ +contract Test { + + function test(bytes memory x) public { + + } + +} \ No newline at end of file diff --git a/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json b/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json new file mode 100644 index 000000000..351921497 --- /dev/null +++ b/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json @@ -0,0 +1,63 @@ +[ + [ + { + "elements": [ + { + "type": "function", + "name": "test", + "source_mapping": { + "start": 21, + "length": 53, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/external-function/0.5.16/external_function_3.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/external-function/0.5.16/external_function_3.sol", + "is_dependency": false, + "lines": [ + 3, + 4, + 5 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Test", + "source_mapping": { + "start": 0, + "length": 77, + "filename_used": "/GENERIC_PATH", + "filename_relative": "tests/detectors/external-function/0.5.16/external_function_3.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/external-function/0.5.16/external_function_3.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "test(bytes)" + } + } + ], + "description": "test(bytes) should be declared external:\n\t- Test.test(bytes) (tests/detectors/external-function/0.5.16/external_function_3.sol#3-5)\n", + "markdown": "test(bytes) should be declared external:\n\t- [Test.test(bytes)](tests/detectors/external-function/0.5.16/external_function_3.sol#L3-L5)\n", + "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function_3.sol#L3-L5", + "id": "058fd57c313c645eff4b4e53234ca0f73ca3362bd28d3fdc90e0219a32099592", + "check": "external-function", + "impact": "Optimization", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/detectors/external-function/0.6.11/external_function.sol.0.6.11.ExternalFunction.json b/tests/detectors/external-function/0.6.11/external_function.sol.0.6.11.ExternalFunction.json index eee61a19f..5825bcacc 100644 --- a/tests/detectors/external-function/0.6.11/external_function.sol.0.6.11.ExternalFunction.json +++ b/tests/detectors/external-function/0.6.11/external_function.sol.0.6.11.ExternalFunction.json @@ -1,342 +1,3 @@ [ - [ - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled3", - "source_mapping": { - "start": 259, - "length": 41, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 13, - 14, - 15 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled3()" - } - } - ], - "description": "funcNotCalled3() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled3() (tests/detectors/external-function/0.6.11/external_function.sol#13-15)\n", - "markdown": "funcNotCalled3() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled3()](tests/detectors/external-function/0.6.11/external_function.sol#L13-L15)\n", - "first_markdown_element": "tests/detectors/external-function/0.6.11/external_function.sol#L13-L15", - "id": "026d9a579ea0304e58c8a5174296494f4b672e4ea032f4e17504f3dac327c4e6", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled2", - "source_mapping": { - "start": 306, - "length": 41, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 17, - 18, - 19 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled2()" - } - } - ], - "description": "funcNotCalled2() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled2() (tests/detectors/external-function/0.6.11/external_function.sol#17-19)\n", - "markdown": "funcNotCalled2() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled2()](tests/detectors/external-function/0.6.11/external_function.sol#L17-L19)\n", - "first_markdown_element": "tests/detectors/external-function/0.6.11/external_function.sol#L17-L19", - "id": "1ef1f19a92a8ab8d27df156d50dd75628ec3057b5f5eb16b7d1faa0e5c3850a0", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 353, - "length": 40, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 21, - 22, - 23 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled() (tests/detectors/external-function/0.6.11/external_function.sol#21-23)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled()](tests/detectors/external-function/0.6.11/external_function.sol#L21-L23)\n", - "first_markdown_element": "tests/detectors/external-function/0.6.11/external_function.sol#L21-L23", - "id": "369a2f3d071735755ff4f5bc43081fe858bbfb07eed94e5c6dda3c8daa22ba26", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 554, - "length": 325, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled2", - "source_mapping": { - "start": 473, - "length": 408, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/external_function.sol", - "is_dependency": false, - "lines": [ - 31, - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39, - 40 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled2.funcNotCalled() (tests/detectors/external-function/0.6.11/external_function.sol#32-39)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled2.funcNotCalled()](tests/detectors/external-function/0.6.11/external_function.sol#L32-L39)\n", - "first_markdown_element": "tests/detectors/external-function/0.6.11/external_function.sol#L32-L39", - "id": "80a0a3a3954cc6e314079a1d8d96d6739d521ddbcf738e63078d7f210e443562", - "check": "external-function", - "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.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/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.6.11/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.6.11/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.6.11/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.6.11/external_function.sol#L74-L76)\n", - "first_markdown_element": "tests/detectors/external-function/0.6.11/external_function.sol#L74-L76", - "id": "3a0a42d128eff9fb04d8f7605bf2d6f7574c2cbbdffa2dcabbae66d7568ecc59", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - } - ] + [] ] \ No newline at end of file diff --git a/tests/detectors/external-function/0.6.11/external_function_3.sol b/tests/detectors/external-function/0.6.11/external_function_3.sol new file mode 100644 index 000000000..641027603 --- /dev/null +++ b/tests/detectors/external-function/0.6.11/external_function_3.sol @@ -0,0 +1,7 @@ +contract Test { + + function test(bytes memory x) public { + + } + +} \ No newline at end of file diff --git a/tests/detectors/external-function/0.6.11/external_function_3.sol.0.6.11.ExternalFunction.json b/tests/detectors/external-function/0.6.11/external_function_3.sol.0.6.11.ExternalFunction.json new file mode 100644 index 000000000..5825bcacc --- /dev/null +++ b/tests/detectors/external-function/0.6.11/external_function_3.sol.0.6.11.ExternalFunction.json @@ -0,0 +1,3 @@ +[ + [] +] \ No newline at end of file diff --git a/tests/detectors/external-function/0.7.6/external_function.sol.0.7.6.ExternalFunction.json b/tests/detectors/external-function/0.7.6/external_function.sol.0.7.6.ExternalFunction.json index 6e90c47ab..5825bcacc 100644 --- a/tests/detectors/external-function/0.7.6/external_function.sol.0.7.6.ExternalFunction.json +++ b/tests/detectors/external-function/0.7.6/external_function.sol.0.7.6.ExternalFunction.json @@ -1,342 +1,3 @@ [ - [ - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled3", - "source_mapping": { - "start": 259, - "length": 41, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 13, - 14, - 15 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled3()" - } - } - ], - "description": "funcNotCalled3() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled3() (tests/detectors/external-function/0.7.6/external_function.sol#13-15)\n", - "markdown": "funcNotCalled3() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled3()](tests/detectors/external-function/0.7.6/external_function.sol#L13-L15)\n", - "first_markdown_element": "tests/detectors/external-function/0.7.6/external_function.sol#L13-L15", - "id": "026d9a579ea0304e58c8a5174296494f4b672e4ea032f4e17504f3dac327c4e6", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled2", - "source_mapping": { - "start": 306, - "length": 41, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 17, - 18, - 19 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled2()" - } - } - ], - "description": "funcNotCalled2() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled2() (tests/detectors/external-function/0.7.6/external_function.sol#17-19)\n", - "markdown": "funcNotCalled2() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled2()](tests/detectors/external-function/0.7.6/external_function.sol#L17-L19)\n", - "first_markdown_element": "tests/detectors/external-function/0.7.6/external_function.sol#L17-L19", - "id": "1ef1f19a92a8ab8d27df156d50dd75628ec3057b5f5eb16b7d1faa0e5c3850a0", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 353, - "length": 40, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 21, - 22, - 23 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled() (tests/detectors/external-function/0.7.6/external_function.sol#21-23)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled()](tests/detectors/external-function/0.7.6/external_function.sol#L21-L23)\n", - "first_markdown_element": "tests/detectors/external-function/0.7.6/external_function.sol#L21-L23", - "id": "369a2f3d071735755ff4f5bc43081fe858bbfb07eed94e5c6dda3c8daa22ba26", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 554, - "length": 325, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled2", - "source_mapping": { - "start": 473, - "length": 408, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/external_function.sol", - "is_dependency": false, - "lines": [ - 31, - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39, - 40 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled2.funcNotCalled() (tests/detectors/external-function/0.7.6/external_function.sol#32-39)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled2.funcNotCalled()](tests/detectors/external-function/0.7.6/external_function.sol#L32-L39)\n", - "first_markdown_element": "tests/detectors/external-function/0.7.6/external_function.sol#L32-L39", - "id": "80a0a3a3954cc6e314079a1d8d96d6739d521ddbcf738e63078d7f210e443562", - "check": "external-function", - "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.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/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.7.6/external_function.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.7.6/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.7.6/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.7.6/external_function.sol#L74-L76)\n", - "first_markdown_element": "tests/detectors/external-function/0.7.6/external_function.sol#L74-L76", - "id": "3a0a42d128eff9fb04d8f7605bf2d6f7574c2cbbdffa2dcabbae66d7568ecc59", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - } - ] + [] ] \ No newline at end of file diff --git a/tests/detectors/external-function/0.7.6/external_function_3.sol b/tests/detectors/external-function/0.7.6/external_function_3.sol new file mode 100644 index 000000000..641027603 --- /dev/null +++ b/tests/detectors/external-function/0.7.6/external_function_3.sol @@ -0,0 +1,7 @@ +contract Test { + + function test(bytes memory x) public { + + } + +} \ No newline at end of file diff --git a/tests/detectors/external-function/0.7.6/external_function_3.sol.0.7.6.ExternalFunction.json b/tests/detectors/external-function/0.7.6/external_function_3.sol.0.7.6.ExternalFunction.json new file mode 100644 index 000000000..5825bcacc --- /dev/null +++ b/tests/detectors/external-function/0.7.6/external_function_3.sol.0.7.6.ExternalFunction.json @@ -0,0 +1,3 @@ +[ + [] +] \ No newline at end of file diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 8d26b12cb..29f143e19 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -487,6 +487,11 @@ ALL_TESTS = [ "external_function_2.sol", "0.4.25", ), + Test( + all_detectors.ExternalFunction, + "external_function_3.sol", + "0.4.25", + ), Test( all_detectors.ExternalFunction, "external_function.sol", @@ -497,6 +502,11 @@ ALL_TESTS = [ "external_function_2.sol", "0.5.16", ), + Test( + all_detectors.ExternalFunction, + "external_function_3.sol", + "0.5.16", + ), Test( all_detectors.ExternalFunction, "external_function.sol", @@ -507,6 +517,11 @@ ALL_TESTS = [ "external_function_2.sol", "0.6.11", ), + Test( + all_detectors.ExternalFunction, + "external_function_3.sol", + "0.6.11", + ), Test( all_detectors.ExternalFunction, "external_function.sol", @@ -517,6 +532,11 @@ ALL_TESTS = [ "external_function_2.sol", "0.7.6", ), + Test( + all_detectors.ExternalFunction, + "external_function_3.sol", + "0.7.6", + ), Test( all_detectors.NamingConvention, "naming_convention.sol", From d0fff8ca9a1cd6642c163e9cd8e1298783729016 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 25 Mar 2022 23:59:15 +0100 Subject: [PATCH 04/12] Apply suggestions from code review Co-authored-by: alpharush <0xalpharush@protonmail.com> --- slither/detectors/functions/external_function.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index c95191ad5..6291d27b3 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -20,7 +20,7 @@ class ExternalFunction(AbstractDetector): WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external" WIKI_TITLE = "Public function that could be declared external" - WIKI_DESCRIPTION = "`public` functions that are never called by the contract should be declared `external` and its immutable parameters should be located in `calldata` to save gas." + WIKI_DESCRIPTION = "`public` functions that are never called by the contract should be declared `external`, and its immutable parameters should be located in `calldata` to save gas." WIKI_RECOMMENDATION = ( "Use the `external` attribute for functions never called from the contract, and change the location of immutable parameters to `calldata` to save gas." ) @@ -110,7 +110,7 @@ class ExternalFunction(AbstractDetector): # After solc 0.6.9, calldata arguments are allowed in public functions if self.compilation_unit.solc_version >= "0.7." or self.compilation_unit.solc_version in ["0.6.9","0.6.10","0.6.11"]: - return [] + return results # Create a set to track contracts with dynamic calls. All contracts with dynamic calls could potentially be # calling functions internally, and thus we can't assume any function in such contracts isn't called by them. From b5db35e7f668aec5a16cf713b0cfb2a6f2ca5c18 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 1 Apr 2022 01:05:34 +0200 Subject: [PATCH 05/12] 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": [ { From 2bf846109901a96bfd655351f598f8fc5096c380 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 1 Apr 2022 15:44:50 +0200 Subject: [PATCH 06/12] Fix: skip functions without args --- .../detectors/functions/external_function.py | 2 +- ..._function.sol.0.4.25.ExternalFunction.json | 279 +----------------- ..._function.sol.0.5.16.ExternalFunction.json | 279 +----------------- 3 files changed, 3 insertions(+), 557 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 16ac9d7f1..4ab2225f4 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -156,7 +156,7 @@ class ExternalFunction(AbstractDetector): 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: + if len(reference_args) == 0: continue # If the function is a constructor, or is public, we skip it. 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 17885c854..5825bcacc 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 @@ -1,280 +1,3 @@ [ - [ - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled3", - "source_mapping": { - "start": 259, - "length": 41, - "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": [ - 13, - 14, - 15 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "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": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled3()" - } - } - ], - "description": "funcNotCalled3() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled3() (tests/detectors/external-function/0.4.25/external_function.sol#13-15)\n", - "markdown": "funcNotCalled3() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled3()](tests/detectors/external-function/0.4.25/external_function.sol#L13-L15)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function.sol#L13-L15", - "id": "026d9a579ea0304e58c8a5174296494f4b672e4ea032f4e17504f3dac327c4e6", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled2", - "source_mapping": { - "start": 306, - "length": 41, - "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": [ - 17, - 18, - 19 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "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": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled2()" - } - } - ], - "description": "funcNotCalled2() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled2() (tests/detectors/external-function/0.4.25/external_function.sol#17-19)\n", - "markdown": "funcNotCalled2() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled2()](tests/detectors/external-function/0.4.25/external_function.sol#L17-L19)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function.sol#L17-L19", - "id": "1ef1f19a92a8ab8d27df156d50dd75628ec3057b5f5eb16b7d1faa0e5c3850a0", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 353, - "length": 40, - "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": [ - 21, - 22, - 23 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "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": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled() (tests/detectors/external-function/0.4.25/external_function.sol#21-23)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled()](tests/detectors/external-function/0.4.25/external_function.sol#L21-L23)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function.sol#L21-L23", - "id": "369a2f3d071735755ff4f5bc43081fe858bbfb07eed94e5c6dda3c8daa22ba26", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 554, - "length": 325, - "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": [ - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled2", - "source_mapping": { - "start": 473, - "length": 408, - "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": [ - 31, - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39, - 40 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled2.funcNotCalled() (tests/detectors/external-function/0.4.25/external_function.sol#32-39)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled2.funcNotCalled()](tests/detectors/external-function/0.4.25/external_function.sol#L32-L39)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function.sol#L32-L39", - "id": "80a0a3a3954cc6e314079a1d8d96d6739d521ddbcf738e63078d7f210e443562", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - } - ] + [] ] \ No newline at end of file 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 3a0514955..5825bcacc 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 @@ -1,280 +1,3 @@ [ - [ - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled3", - "source_mapping": { - "start": 259, - "length": 41, - "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": [ - 13, - 14, - 15 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "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": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled3()" - } - } - ], - "description": "funcNotCalled3() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled3() (tests/detectors/external-function/0.5.16/external_function.sol#13-15)\n", - "markdown": "funcNotCalled3() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled3()](tests/detectors/external-function/0.5.16/external_function.sol#L13-L15)\n", - "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function.sol#L13-L15", - "id": "026d9a579ea0304e58c8a5174296494f4b672e4ea032f4e17504f3dac327c4e6", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled2", - "source_mapping": { - "start": 306, - "length": 41, - "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": [ - 17, - 18, - 19 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "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": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled2()" - } - } - ], - "description": "funcNotCalled2() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled2() (tests/detectors/external-function/0.5.16/external_function.sol#17-19)\n", - "markdown": "funcNotCalled2() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled2()](tests/detectors/external-function/0.5.16/external_function.sol#L17-L19)\n", - "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function.sol#L17-L19", - "id": "1ef1f19a92a8ab8d27df156d50dd75628ec3057b5f5eb16b7d1faa0e5c3850a0", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 353, - "length": 40, - "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": [ - 21, - 22, - 23 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled", - "source_mapping": { - "start": 213, - "length": 258, - "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": [ - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25, - 26, - 27, - 28, - 29 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled() (tests/detectors/external-function/0.5.16/external_function.sol#21-23)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled.funcNotCalled()](tests/detectors/external-function/0.5.16/external_function.sol#L21-L23)\n", - "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function.sol#L21-L23", - "id": "369a2f3d071735755ff4f5bc43081fe858bbfb07eed94e5c6dda3c8daa22ba26", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "funcNotCalled", - "source_mapping": { - "start": 554, - "length": 325, - "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": [ - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "ContractWithFunctionNotCalled2", - "source_mapping": { - "start": 473, - "length": 408, - "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": [ - 31, - 32, - 33, - 34, - 35, - 36, - 37, - 38, - 39, - 40 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "funcNotCalled()" - } - } - ], - "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled2.funcNotCalled() (tests/detectors/external-function/0.5.16/external_function.sol#32-39)\n", - "markdown": "funcNotCalled() should be declared external:\n\t- [ContractWithFunctionNotCalled2.funcNotCalled()](tests/detectors/external-function/0.5.16/external_function.sol#L32-L39)\n", - "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function.sol#L32-L39", - "id": "80a0a3a3954cc6e314079a1d8d96d6739d521ddbcf738e63078d7f210e443562", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - } - ] + [] ] \ No newline at end of file From b31c73de3d27f2800a126e28eff2d75058955ff3 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 1 Apr 2022 21:27:54 +0200 Subject: [PATCH 07/12] Linting --- .../detectors/functions/external_function.py | 22 +++++++++++-------- tests/test_detectors.py | 8 +++---- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 4ab2225f4..231560014 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -25,9 +25,7 @@ class ExternalFunction(AbstractDetector): WIKI_TITLE = "Public function that could be declared external" WIKI_DESCRIPTION = "`public` functions that are never called by the contract should be declared `external`, and its immutable parameters should be located in `calldata` to save gas." - WIKI_RECOMMENDATION = ( - "Use the `external` attribute for functions never called from the contract, and change the location of immutable parameters to `calldata` to save gas." - ) + WIKI_RECOMMENDATION = "Use the `external` attribute for functions never called from the contract, and change the location of immutable parameters to `calldata` to save gas." @staticmethod def detect_functions_called(contract): @@ -112,10 +110,12 @@ class ExternalFunction(AbstractDetector): @staticmethod def is_reference_type(parameter): if isinstance(parameter.type, ArrayType): - return True + return True if isinstance(parameter.type, MappingType): return True - if isinstance(parameter.type, UserDefinedType) and isinstance(parameter.type.type, Structure): + if isinstance(parameter.type, UserDefinedType) and isinstance( + parameter.type.type, Structure + ): return True if str(parameter.type) in ["bytes", "string"]: return True @@ -125,7 +125,11 @@ class ExternalFunction(AbstractDetector): results = [] # After solc 0.6.9, calldata arguments are allowed in public functions - if self.compilation_unit.solc_version >= "0.7." or self.compilation_unit.solc_version in ["0.6.9","0.6.10","0.6.11"]: + if self.compilation_unit.solc_version >= "0.7." or self.compilation_unit.solc_version in [ + "0.6.9", + "0.6.10", + "0.6.11", + ]: return results # Create a set to track contracts with dynamic calls. All contracts with dynamic calls could potentially be @@ -151,13 +155,13 @@ 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. + # 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': + if self.is_reference_type(arg) and arg.location == "memory": reference_args.append(arg) if len(reference_args) == 0: - continue + continue # If the function is a constructor, or is public, we skip it. if function.is_constructor or function.visibility != "public": diff --git a/tests/test_detectors.py b/tests/test_detectors.py index e55740431..5aec956a5 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -514,7 +514,7 @@ ALL_TEST_OBJECTS = [ all_detectors.ExternalFunction, "external_function_3.sol", "0.4.25", - ), + ), Test( all_detectors.ExternalFunction, "external_function.sol", @@ -529,7 +529,7 @@ ALL_TEST_OBJECTS = [ all_detectors.ExternalFunction, "external_function_3.sol", "0.5.16", - ), + ), Test( all_detectors.ExternalFunction, "external_function.sol", @@ -544,7 +544,7 @@ ALL_TEST_OBJECTS = [ all_detectors.ExternalFunction, "external_function_3.sol", "0.6.11", - ), + ), Test( all_detectors.ExternalFunction, "external_function.sol", @@ -559,7 +559,7 @@ ALL_TEST_OBJECTS = [ all_detectors.ExternalFunction, "external_function_3.sol", "0.7.6", - ), + ), Test( all_detectors.NamingConvention, "naming_convention.sol", From 166eaed76178acd7657a773e25b37e68545fb9ff Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 1 Apr 2022 21:37:47 +0200 Subject: [PATCH 08/12] Improved tests --- .../0.4.25/external_function_3.sol | 11 +- ...unction_3.sol.0.4.25.ExternalFunction.json | 129 +----------------- .../0.5.16/external_function_3.sol | 9 +- ...unction_3.sol.0.5.16.ExternalFunction.json | 62 +-------- .../0.6.11/external_function_3.sol | 9 +- .../0.7.6/external_function_3.sol | 9 +- 6 files changed, 25 insertions(+), 204 deletions(-) diff --git a/tests/detectors/external-function/0.4.25/external_function_3.sol b/tests/detectors/external-function/0.4.25/external_function_3.sol index 2ea376fcd..eb11475e5 100644 --- a/tests/detectors/external-function/0.4.25/external_function_3.sol +++ b/tests/detectors/external-function/0.4.25/external_function_3.sol @@ -1,11 +1,10 @@ contract Test { - function test(bytes x) public { - - } + function good(bytes x) external {} + function good2() public {} - function test2(bytes memory x) public { - - } + function good3(uint256 x, uint256 y) public {} + + function good4(uint256[] x, string y) external {} } \ No newline at end of file diff --git a/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json b/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json index c7c7ae31e..5825bcacc 100644 --- a/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json +++ b/tests/detectors/external-function/0.4.25/external_function_3.sol.0.4.25.ExternalFunction.json @@ -1,130 +1,3 @@ [ - [ - { - "elements": [ - { - "type": "function", - "name": "test", - "source_mapping": { - "start": 21, - "length": 46, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "is_dependency": false, - "lines": [ - 3, - 4, - 5 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "Test", - "source_mapping": { - "start": 0, - "length": 134, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "is_dependency": false, - "lines": [ - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - 9, - 10, - 11, - 12 - ], - "starting_column": 1, - "ending_column": 0 - } - }, - "signature": "test(bytes)" - } - } - ], - "description": "test(bytes) should be declared external:\n\t- Test.test(bytes) (tests/detectors/external-function/0.4.25/external_function_3.sol#3-5)\n", - "markdown": "test(bytes) should be declared external:\n\t- [Test.test(bytes)](tests/detectors/external-function/0.4.25/external_function_3.sol#L3-L5)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function_3.sol#L3-L5", - "id": "058fd57c313c645eff4b4e53234ca0f73ca3362bd28d3fdc90e0219a32099592", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - }, - { - "elements": [ - { - "type": "function", - "name": "test2", - "source_mapping": { - "start": 73, - "length": 54, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "is_dependency": false, - "lines": [ - 7, - 8, - 9 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "Test", - "source_mapping": { - "start": 0, - "length": 134, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.4.25/external_function_3.sol", - "is_dependency": false, - "lines": [ - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - 9, - 10, - 11, - 12 - ], - "starting_column": 1, - "ending_column": 0 - } - }, - "signature": "test2(bytes)" - } - } - ], - "description": "test2(bytes) should be declared external:\n\t- Test.test2(bytes) (tests/detectors/external-function/0.4.25/external_function_3.sol#7-9)\n", - "markdown": "test2(bytes) should be declared external:\n\t- [Test.test2(bytes)](tests/detectors/external-function/0.4.25/external_function_3.sol#L7-L9)\n", - "first_markdown_element": "tests/detectors/external-function/0.4.25/external_function_3.sol#L7-L9", - "id": "a5b4753f43bb5a2a669ecbf4ce6ab1aaff060454657b16b5ed8cc9c34b521c79", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - } - ] + [] ] \ No newline at end of file diff --git a/tests/detectors/external-function/0.5.16/external_function_3.sol b/tests/detectors/external-function/0.5.16/external_function_3.sol index 641027603..92d6bcc5e 100644 --- a/tests/detectors/external-function/0.5.16/external_function_3.sol +++ b/tests/detectors/external-function/0.5.16/external_function_3.sol @@ -1,7 +1,10 @@ contract Test { - function test(bytes memory x) public { - - } + function good(bytes calldata x) external {} + function good2() public {} + + function good3(uint256 x, uint256 y) public {} + + function good4(uint256[] calldata x, string calldata y) external {} } \ No newline at end of file diff --git a/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json b/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json index 351921497..5825bcacc 100644 --- a/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json +++ b/tests/detectors/external-function/0.5.16/external_function_3.sol.0.5.16.ExternalFunction.json @@ -1,63 +1,3 @@ [ - [ - { - "elements": [ - { - "type": "function", - "name": "test", - "source_mapping": { - "start": 21, - "length": 53, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.5.16/external_function_3.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.5.16/external_function_3.sol", - "is_dependency": false, - "lines": [ - 3, - 4, - 5 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "Test", - "source_mapping": { - "start": 0, - "length": 77, - "filename_used": "/GENERIC_PATH", - "filename_relative": "tests/detectors/external-function/0.5.16/external_function_3.sol", - "filename_absolute": "/GENERIC_PATH", - "filename_short": "tests/detectors/external-function/0.5.16/external_function_3.sol", - "is_dependency": false, - "lines": [ - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8 - ], - "starting_column": 1, - "ending_column": 0 - } - }, - "signature": "test(bytes)" - } - } - ], - "description": "test(bytes) should be declared external:\n\t- Test.test(bytes) (tests/detectors/external-function/0.5.16/external_function_3.sol#3-5)\n", - "markdown": "test(bytes) should be declared external:\n\t- [Test.test(bytes)](tests/detectors/external-function/0.5.16/external_function_3.sol#L3-L5)\n", - "first_markdown_element": "tests/detectors/external-function/0.5.16/external_function_3.sol#L3-L5", - "id": "058fd57c313c645eff4b4e53234ca0f73ca3362bd28d3fdc90e0219a32099592", - "check": "external-function", - "impact": "Optimization", - "confidence": "High" - } - ] + [] ] \ No newline at end of file diff --git a/tests/detectors/external-function/0.6.11/external_function_3.sol b/tests/detectors/external-function/0.6.11/external_function_3.sol index 641027603..92d6bcc5e 100644 --- a/tests/detectors/external-function/0.6.11/external_function_3.sol +++ b/tests/detectors/external-function/0.6.11/external_function_3.sol @@ -1,7 +1,10 @@ contract Test { - function test(bytes memory x) public { - - } + function good(bytes calldata x) external {} + function good2() public {} + + function good3(uint256 x, uint256 y) public {} + + function good4(uint256[] calldata x, string calldata y) external {} } \ No newline at end of file diff --git a/tests/detectors/external-function/0.7.6/external_function_3.sol b/tests/detectors/external-function/0.7.6/external_function_3.sol index 641027603..92d6bcc5e 100644 --- a/tests/detectors/external-function/0.7.6/external_function_3.sol +++ b/tests/detectors/external-function/0.7.6/external_function_3.sol @@ -1,7 +1,10 @@ contract Test { - function test(bytes memory x) public { - - } + function good(bytes calldata x) external {} + function good2() public {} + + function good3(uint256 x, uint256 y) public {} + + function good4(uint256[] calldata x, string calldata y) external {} } \ No newline at end of file From 767094fff12504437cef501c3415919adb551e18 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 15 Apr 2022 20:35:59 +0200 Subject: [PATCH 09/12] Removed detection of mapping type --- slither/detectors/functions/external_function.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 231560014..67c42d2d1 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -1,6 +1,5 @@ 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 @@ -111,8 +110,6 @@ class ExternalFunction(AbstractDetector): 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 ): From f295bf35f83cb8153d871566fa6792eeffb31fe5 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 15 Apr 2022 20:59:12 +0200 Subject: [PATCH 10/12] Increased tests coverage --- .../external-function/0.4.25/external_function_3.sol | 2 -- .../external-function/0.5.16/external_function_3.sol | 10 ++++++++-- .../external-function/0.6.11/external_function_3.sol | 10 ++++++++-- .../external-function/0.7.6/external_function_3.sol | 10 ++++++++-- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/detectors/external-function/0.4.25/external_function_3.sol b/tests/detectors/external-function/0.4.25/external_function_3.sol index eb11475e5..1e215bceb 100644 --- a/tests/detectors/external-function/0.4.25/external_function_3.sol +++ b/tests/detectors/external-function/0.4.25/external_function_3.sol @@ -2,9 +2,7 @@ contract Test { function good(bytes x) external {} function good2() public {} - function good3(uint256 x, uint256 y) public {} - function good4(uint256[] x, string y) external {} } \ No newline at end of file diff --git a/tests/detectors/external-function/0.5.16/external_function_3.sol b/tests/detectors/external-function/0.5.16/external_function_3.sol index 92d6bcc5e..3e9204c96 100644 --- a/tests/detectors/external-function/0.5.16/external_function_3.sol +++ b/tests/detectors/external-function/0.5.16/external_function_3.sol @@ -1,10 +1,16 @@ +pragma experimental ABIEncoderV2; + contract Test { + struct testStruct { + uint256 id; + string name; + } + function good(bytes calldata x) external {} function good2() public {} - function good3(uint256 x, uint256 y) public {} - function good4(uint256[] calldata x, string calldata y) external {} + function good5(testStruct calldata x) external {} } \ No newline at end of file diff --git a/tests/detectors/external-function/0.6.11/external_function_3.sol b/tests/detectors/external-function/0.6.11/external_function_3.sol index 92d6bcc5e..3e9204c96 100644 --- a/tests/detectors/external-function/0.6.11/external_function_3.sol +++ b/tests/detectors/external-function/0.6.11/external_function_3.sol @@ -1,10 +1,16 @@ +pragma experimental ABIEncoderV2; + contract Test { + struct testStruct { + uint256 id; + string name; + } + function good(bytes calldata x) external {} function good2() public {} - function good3(uint256 x, uint256 y) public {} - function good4(uint256[] calldata x, string calldata y) external {} + function good5(testStruct calldata x) external {} } \ No newline at end of file diff --git a/tests/detectors/external-function/0.7.6/external_function_3.sol b/tests/detectors/external-function/0.7.6/external_function_3.sol index 92d6bcc5e..3e9204c96 100644 --- a/tests/detectors/external-function/0.7.6/external_function_3.sol +++ b/tests/detectors/external-function/0.7.6/external_function_3.sol @@ -1,10 +1,16 @@ +pragma experimental ABIEncoderV2; + contract Test { + struct testStruct { + uint256 id; + string name; + } + function good(bytes calldata x) external {} function good2() public {} - function good3(uint256 x, uint256 y) public {} - function good4(uint256[] calldata x, string calldata y) external {} + function good5(testStruct calldata x) external {} } \ No newline at end of file From d28df4df3fef46fc2e6b575afb8a89f1110e7205 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 15 Apr 2022 23:20:11 +0200 Subject: [PATCH 11/12] Fixed wrong import --- slither/detectors/functions/external_function.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 67c42d2d1..5344a6881 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -1,4 +1,4 @@ -from array import ArrayType +from slither.core.solidity_types.array_type import ArrayType from slither.core.declarations.structure import Structure from slither.core.solidity_types.user_defined_type import UserDefinedType from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification From c878a8fc25b8a82fb41b4400f2c97f0ec6061fc6 Mon Sep 17 00:00:00 2001 From: Tadashi Date: Fri, 15 Apr 2022 23:22:13 +0200 Subject: [PATCH 12/12] Added parameters that should be changed in info --- slither/detectors/functions/external_function.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index 5344a6881..a24bc0e93 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -232,6 +232,12 @@ class ExternalFunction(AbstractDetector): info = [f"{function_definition.full_name} should be declared external:\n"] info += ["\t- ", function_definition, "\n"] + if self.compilation_unit.solc_version >= "0.5.": + info += [ + "Moreover, the following function parameters should change its data location:\n" + ] + for reference_arg in reference_args: + info += [f"{reference_arg} location should be calldata\n"] for other_function_definition in all_function_definitions: info += ["\t- ", other_function_definition, "\n"]