From 2440b95fe8331d598e19f8f2b5bbc7ddb28ce151 Mon Sep 17 00:00:00 2001 From: Vlad Silviu Farcas Date: Wed, 27 Nov 2019 09:52:12 +0000 Subject: [PATCH 1/6] Fix tests --- scripts/tests_generate_expected_json_4.sh | 3 +- scripts/tests_generate_expected_json_5.sh | 3 +- scripts/travis_test_4.sh | 3 +- scripts/travis_test_5.sh | 3 +- .../attributes/const_functions_asm.py | 2 +- .../attributes/const_functions_state.py | 2 +- slither/tools/slither_format/__main__.py | 3 +- ...constant-0.5.1.constant-function-asm.json} | 0 .../constant-0.5.1.constant-function-asm.txt | 1 + ...onstant-0.5.1.constant-function-state.json | 5 ++ ...constant-0.5.1.constant-function-state.txt | 0 .../constant-0.5.1.constant-function.txt | 4 - .../constant.constant-function-asm.json | 86 +++++++++++++++++++ .../constant.constant-function-asm.txt | 4 + ... => constant.constant-function-state.json} | 78 ----------------- ...t => constant.constant-function-state.txt} | 1 - 16 files changed, 108 insertions(+), 90 deletions(-) rename tests/expected_json/{constant-0.5.1.constant-function.json => constant-0.5.1.constant-function-asm.json} (100%) create mode 100644 tests/expected_json/constant-0.5.1.constant-function-asm.txt create mode 100644 tests/expected_json/constant-0.5.1.constant-function-state.json create mode 100644 tests/expected_json/constant-0.5.1.constant-function-state.txt delete mode 100644 tests/expected_json/constant-0.5.1.constant-function.txt create mode 100644 tests/expected_json/constant.constant-function-asm.json create mode 100644 tests/expected_json/constant.constant-function-asm.txt rename tests/expected_json/{constant.constant-function.json => constant.constant-function-state.json} (78%) rename tests/expected_json/{constant.constant-function.txt => constant.constant-function-state.txt} (82%) diff --git a/scripts/tests_generate_expected_json_4.sh b/scripts/tests_generate_expected_json_4.sh index c44898d9f..302f318b3 100755 --- a/scripts/tests_generate_expected_json_4.sh +++ b/scripts/tests_generate_expected_json_4.sh @@ -46,7 +46,8 @@ generate_expected_json tests/reentrancy.sol "reentrancy-unlimited-gas" #generate_expected_json tests/naming_convention.sol "naming-convention" #generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local" #generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall" -#generate_expected_json tests/constant.sol "constant-function" +#generate_expected_json tests/constant.sol "constant-function-asm" +#generate_expected_json tests/constant.sol "constant-function-state" #generate_expected_json tests/unused_return.sol "unused-return" #generate_expected_json tests/shadowing_state_variable.sol "shadowing-state" #generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract" diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh index 620fc0b0c..8804974a5 100755 --- a/scripts/tests_generate_expected_json_5.sh +++ b/scripts/tests_generate_expected_json_5.sh @@ -32,7 +32,8 @@ generate_expected_json(){ #generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send" #generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly" #generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly" -#generate_expected_json tests/constant-0.5.1.sol "constant-function" +#generate_expected_json tests/constant-0.5.1.sol "constant-function-asm" +#generate_expected_json tests/constant-0.5.1.sol "constant-function-state" #generate_expected_json tests/incorrect_equality.sol "incorrect-equality" #generate_expected_json tests/too_many_digits.sol "too-many-digits" #generate_expected_json tests/unchecked_lowlevel-0.5.1.sol "unchecked-lowlevel" diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh index 01e5088df..aae98669e 100755 --- a/scripts/travis_test_4.sh +++ b/scripts/travis_test_4.sh @@ -95,7 +95,8 @@ test_slither tests/naming_convention.sol "naming-convention" #test_slither tests/complex_func.sol "complex-function" test_slither tests/controlled_delegatecall.sol "controlled-delegatecall" test_slither tests/uninitialized_local_variable.sol "uninitialized-local" -test_slither tests/constant.sol "constant-function" +test_slither tests/constant.sol "constant-function-asm" +test_slither tests/constant.sol "constant-function-state" test_slither tests/unused_return.sol "unused-return" test_slither tests/shadowing_abstract.sol "shadowing-abstract" test_slither tests/shadowing_state_variable.sol "shadowing-state" diff --git a/scripts/travis_test_5.sh b/scripts/travis_test_5.sh index 4b3928b5c..d0f91b1b4 100755 --- a/scripts/travis_test_5.sh +++ b/scripts/travis_test_5.sh @@ -91,7 +91,8 @@ test_slither tests/external_function_2.sol "external-function" test_slither tests/naming_convention.sol "naming-convention" #test_slither tests/complex_func.sol "complex-function" test_slither tests/controlled_delegatecall.sol "controlled-delegatecall" -test_slither tests/constant-0.5.1.sol "constant-function" +test_slither tests/constant-0.5.1.sol "constant-function-asm" +test_slither tests/constant-0.5.1.sol "constant-function-state" test_slither tests/unused_return.sol "unused-return" test_slither tests/timestamp.sol "timestamp" test_slither tests/incorrect_equality.sol "incorrect-equality" diff --git a/slither/detectors/attributes/const_functions_asm.py b/slither/detectors/attributes/const_functions_asm.py index fae375d09..89af4d9ce 100644 --- a/slither/detectors/attributes/const_functions_asm.py +++ b/slither/detectors/attributes/const_functions_asm.py @@ -11,7 +11,7 @@ class ConstantFunctionsAsm(AbstractDetector): Constant function detector """ - ARGUMENT = 'constant-function' # run the detector with slither.py --ARGUMENT + ARGUMENT = 'constant-function-asm' # run the detector with slither.py --ARGUMENT HELP = 'Constant functions changing the state' # help information IMPACT = DetectorClassification.MEDIUM CONFIDENCE = DetectorClassification.MEDIUM diff --git a/slither/detectors/attributes/const_functions_state.py b/slither/detectors/attributes/const_functions_state.py index 919edc198..6b39be702 100644 --- a/slither/detectors/attributes/const_functions_state.py +++ b/slither/detectors/attributes/const_functions_state.py @@ -11,7 +11,7 @@ class ConstantFunctionsState(AbstractDetector): Constant function detector """ - ARGUMENT = 'constant-function' # run the detector with slither.py --ARGUMENT + ARGUMENT = 'constant-function-state' # run the detector with slither.py --ARGUMENT HELP = 'Constant functions changing the state' # help information IMPACT = DetectorClassification.MEDIUM CONFIDENCE = DetectorClassification.MEDIUM diff --git a/slither/tools/slither_format/__main__.py b/slither/tools/slither_format/__main__.py index 82256a5fa..66787ee0e 100644 --- a/slither/tools/slither_format/__main__.py +++ b/slither/tools/slither_format/__main__.py @@ -16,7 +16,8 @@ available_detectors = ["unused-state", "naming-convention", "external-function", "constable-states", - "constant-function"] + "constant-function-asm", + "constatnt-function-state"] detectors_to_run = [] diff --git a/tests/expected_json/constant-0.5.1.constant-function.json b/tests/expected_json/constant-0.5.1.constant-function-asm.json similarity index 100% rename from tests/expected_json/constant-0.5.1.constant-function.json rename to tests/expected_json/constant-0.5.1.constant-function-asm.json diff --git a/tests/expected_json/constant-0.5.1.constant-function-asm.txt b/tests/expected_json/constant-0.5.1.constant-function-asm.txt new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/tests/expected_json/constant-0.5.1.constant-function-asm.txt @@ -0,0 +1 @@ + diff --git a/tests/expected_json/constant-0.5.1.constant-function-state.json b/tests/expected_json/constant-0.5.1.constant-function-state.json new file mode 100644 index 000000000..c5488496b --- /dev/null +++ b/tests/expected_json/constant-0.5.1.constant-function-state.json @@ -0,0 +1,5 @@ +{ + "success": true, + "error": null, + "results": null +} \ No newline at end of file diff --git a/tests/expected_json/constant-0.5.1.constant-function-state.txt b/tests/expected_json/constant-0.5.1.constant-function-state.txt new file mode 100644 index 000000000..e69de29bb diff --git a/tests/expected_json/constant-0.5.1.constant-function.txt b/tests/expected_json/constant-0.5.1.constant-function.txt deleted file mode 100644 index 1fa22f64d..000000000 --- a/tests/expected_json/constant-0.5.1.constant-function.txt +++ /dev/null @@ -1,4 +0,0 @@ - -Constant.test_assembly_bug() (tests/constant-0.5.1.sol#15-17) is declared view but contains assembly code -Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state -tests/constant-0.5.1.sol analyzed (1 contracts with 1 detectors), 1 result(s) found diff --git a/tests/expected_json/constant.constant-function-asm.json b/tests/expected_json/constant.constant-function-asm.json new file mode 100644 index 000000000..bc754a1ac --- /dev/null +++ b/tests/expected_json/constant.constant-function-asm.json @@ -0,0 +1,86 @@ +{ + "success": true, + "error": null, + "results": { + "detectors": [ + { + "elements": [ + { + "type": "function", + "name": "test_assembly_bug", + "source_mapping": { + "start": 324, + "length": 66, + "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol", + "filename_relative": "tests/constant.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol", + "filename_short": "tests/constant.sol", + "is_dependency": false, + "lines": [ + 22, + 23, + 24 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Constant", + "source_mapping": { + "start": 0, + "length": 392, + "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol", + "filename_relative": "tests/constant.sol", + "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol", + "filename_short": "tests/constant.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 17, + 18, + 19, + 20, + 21, + 22, + 23, + 24, + 25 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "test_assembly_bug()" + } + } + ], + "description": "Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code\n", + "markdown": "[Constant.test_assembly_bug()](tests/constant.sol#L22-L24) is declared view but contains assembly code\n", + "id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23", + "additional_fields": { + "contains_assembly": true + }, + "check": "constant-function-asm", + "impact": "Medium", + "confidence": "Medium" + } + ] + } +} \ No newline at end of file diff --git a/tests/expected_json/constant.constant-function-asm.txt b/tests/expected_json/constant.constant-function-asm.txt new file mode 100644 index 000000000..19daeb137 --- /dev/null +++ b/tests/expected_json/constant.constant-function-asm.txt @@ -0,0 +1,4 @@ + +Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code +Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state +tests/constant.sol analyzed (1 contracts with 1 detectors), 3 result(s) found diff --git a/tests/expected_json/constant.constant-function.json b/tests/expected_json/constant.constant-function-state.json similarity index 78% rename from tests/expected_json/constant.constant-function.json rename to tests/expected_json/constant.constant-function-state.json index fe42996de..ff6a558b3 100644 --- a/tests/expected_json/constant.constant-function.json +++ b/tests/expected_json/constant.constant-function-state.json @@ -282,84 +282,6 @@ "check": "constant-function-state", "impact": "Medium", "confidence": "Medium" - }, - { - "elements": [ - { - "type": "function", - "name": "test_assembly_bug", - "source_mapping": { - "start": 324, - "length": 66, - "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol", - "filename_relative": "tests/constant.sol", - "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol", - "filename_short": "tests/constant.sol", - "is_dependency": false, - "lines": [ - 22, - 23, - 24 - ], - "starting_column": 5, - "ending_column": 6 - }, - "type_specific_fields": { - "parent": { - "type": "contract", - "name": "Constant", - "source_mapping": { - "start": 0, - "length": 392, - "filename_used": "/home/travis/build/crytic/slither/tests/constant.sol", - "filename_relative": "tests/constant.sol", - "filename_absolute": "/home/travis/build/crytic/slither/tests/constant.sol", - "filename_short": "tests/constant.sol", - "is_dependency": false, - "lines": [ - 1, - 2, - 3, - 4, - 5, - 6, - 7, - 8, - 9, - 10, - 11, - 12, - 13, - 14, - 15, - 16, - 17, - 18, - 19, - 20, - 21, - 22, - 23, - 24, - 25 - ], - "starting_column": 1, - "ending_column": 2 - } - }, - "signature": "test_assembly_bug()" - } - } - ], - "description": "Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code\n", - "markdown": "[Constant.test_assembly_bug()](tests/constant.sol#L22-L24) is declared view but contains assembly code\n", - "id": "1f892cae08b89096bdc4d6ecdf55a3adc4b4314390e054fe2547d9c8e9f76e23", - "additional_fields": { - "contains_assembly": true - }, - "check": "constant-function-asm", - "impact": "Medium", - "confidence": "Medium" } ] } diff --git a/tests/expected_json/constant.constant-function.txt b/tests/expected_json/constant.constant-function-state.txt similarity index 82% rename from tests/expected_json/constant.constant-function.txt rename to tests/expected_json/constant.constant-function-state.txt index f2b695232..5c58c0f83 100644 --- a/tests/expected_json/constant.constant-function.txt +++ b/tests/expected_json/constant.constant-function-state.txt @@ -3,6 +3,5 @@ Constant.test_view_bug() (tests/constant.sol#5-7) is declared view but changes s - Constant.a (tests/constant.sol#3) Constant.test_constant_bug() (tests/constant.sol#9-11) is declared view but changes state variables: - Constant.a (tests/constant.sol#3) -Constant.test_assembly_bug() (tests/constant.sol#22-24) is declared view but contains assembly code Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#constant-functions-changing-the-state tests/constant.sol analyzed (1 contracts with 1 detectors), 3 result(s) found From 3fbd78e3a7313a36c6a5bb2fbcc765dd1d35a355 Mon Sep 17 00:00:00 2001 From: Vlad Silviu Farcas Date: Wed, 27 Nov 2019 10:04:10 +0000 Subject: [PATCH 2/6] Fix tests --- slither/detectors/attributes/const_functions_asm.py | 2 +- slither/detectors/attributes/const_functions_state.py | 2 +- tests/expected_json/constant-0.5.1.constant-function-state.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/slither/detectors/attributes/const_functions_asm.py b/slither/detectors/attributes/const_functions_asm.py index 89af4d9ce..5bf0bacfe 100644 --- a/slither/detectors/attributes/const_functions_asm.py +++ b/slither/detectors/attributes/const_functions_asm.py @@ -50,7 +50,7 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' list: {'vuln', 'filename,'contract','func','#varsWritten'} """ results = [] - if self.slither.solc_version < "0.5.0": + if self.slither._solc_version < "0.5.0": for c in self.contracts: for f in c.functions: if f.contract_declarer != c: diff --git a/slither/detectors/attributes/const_functions_state.py b/slither/detectors/attributes/const_functions_state.py index 6b39be702..0bf8fdd5f 100644 --- a/slither/detectors/attributes/const_functions_state.py +++ b/slither/detectors/attributes/const_functions_state.py @@ -50,7 +50,7 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' list: {'vuln', 'filename,'contract','func','#varsWritten'} """ results = [] - if self.slither.solc_version < "0.5.0": + if self.slither._solc_version < "0.5.0": for c in self.contracts: for f in c.functions: if f.contract_declarer != c: diff --git a/tests/expected_json/constant-0.5.1.constant-function-state.json b/tests/expected_json/constant-0.5.1.constant-function-state.json index c5488496b..f8c07c1d8 100644 --- a/tests/expected_json/constant-0.5.1.constant-function-state.json +++ b/tests/expected_json/constant-0.5.1.constant-function-state.json @@ -1,5 +1,5 @@ { "success": true, "error": null, - "results": null + "results": {} } \ No newline at end of file From 6fdfeb85b1162b318bceca1c3101ee7cc7c67a18 Mon Sep 17 00:00:00 2001 From: Vlad Silviu Farcas Date: Wed, 27 Nov 2019 10:17:01 +0000 Subject: [PATCH 3/6] Fix tests --- tests/expected_json/constant-0.5.1.constant-function-asm.json | 2 +- tests/expected_json/constant-0.5.1.constant-function-asm.txt | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/expected_json/constant-0.5.1.constant-function-asm.json b/tests/expected_json/constant-0.5.1.constant-function-asm.json index c5488496b..f8c07c1d8 100644 --- a/tests/expected_json/constant-0.5.1.constant-function-asm.json +++ b/tests/expected_json/constant-0.5.1.constant-function-asm.json @@ -1,5 +1,5 @@ { "success": true, "error": null, - "results": null + "results": {} } \ No newline at end of file diff --git a/tests/expected_json/constant-0.5.1.constant-function-asm.txt b/tests/expected_json/constant-0.5.1.constant-function-asm.txt index 8b1378917..e69de29bb 100644 --- a/tests/expected_json/constant-0.5.1.constant-function-asm.txt +++ b/tests/expected_json/constant-0.5.1.constant-function-asm.txt @@ -1 +0,0 @@ - From 5cce160c4a75fbc43c182df5ca74e44aef2a9599 Mon Sep 17 00:00:00 2001 From: Vlad Silviu Farcas Date: Wed, 27 Nov 2019 10:22:57 +0000 Subject: [PATCH 4/6] Fix tests --- tests/constant-0.5.1.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/constant-0.5.1.sol b/tests/constant-0.5.1.sol index 3293ab45d..fd3a4a192 100644 --- a/tests/constant-0.5.1.sol +++ b/tests/constant-0.5.1.sol @@ -1,3 +1,5 @@ +pragma solidity 0.5.1; + contract Constant { uint a; From f4413c097eb47eea1a8a4e848992da29ef27c014 Mon Sep 17 00:00:00 2001 From: Josselin Date: Thu, 28 Nov 2019 12:35:32 +0100 Subject: [PATCH 5/6] Use slither.solc_version instead of slither._solc_version --- slither/detectors/attributes/const_functions_asm.py | 2 +- slither/detectors/attributes/const_functions_state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/detectors/attributes/const_functions_asm.py b/slither/detectors/attributes/const_functions_asm.py index 5bf0bacfe..89af4d9ce 100644 --- a/slither/detectors/attributes/const_functions_asm.py +++ b/slither/detectors/attributes/const_functions_asm.py @@ -50,7 +50,7 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' list: {'vuln', 'filename,'contract','func','#varsWritten'} """ results = [] - if self.slither._solc_version < "0.5.0": + if self.slither.solc_version < "0.5.0": for c in self.contracts: for f in c.functions: if f.contract_declarer != c: diff --git a/slither/detectors/attributes/const_functions_state.py b/slither/detectors/attributes/const_functions_state.py index 0bf8fdd5f..6b39be702 100644 --- a/slither/detectors/attributes/const_functions_state.py +++ b/slither/detectors/attributes/const_functions_state.py @@ -50,7 +50,7 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' list: {'vuln', 'filename,'contract','func','#varsWritten'} """ results = [] - if self.slither._solc_version < "0.5.0": + if self.slither.solc_version < "0.5.0": for c in self.contracts: for f in c.functions: if f.contract_declarer != c: From a0b8ffcbd655098c74634b8484755f63099368aa Mon Sep 17 00:00:00 2001 From: Josselin Date: Thu, 28 Nov 2019 12:49:30 +0100 Subject: [PATCH 6/6] Handle missing self.slither.solc_version --- .../attributes/const_functions_asm.py | 23 ++++++++------- .../attributes/const_functions_state.py | 29 ++++++++++--------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/slither/detectors/attributes/const_functions_asm.py b/slither/detectors/attributes/const_functions_asm.py index 89af4d9ce..2b9486f96 100644 --- a/slither/detectors/attributes/const_functions_asm.py +++ b/slither/detectors/attributes/const_functions_asm.py @@ -50,19 +50,20 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' list: {'vuln', 'filename,'contract','func','#varsWritten'} """ results = [] - if self.slither.solc_version < "0.5.0": - for c in self.contracts: - for f in c.functions: - if f.contract_declarer != c: - continue - if f.view or f.pure: - if f.contains_assembly: - attr = 'view' if f.view else 'pure' + if self.slither.solc_version and self.slither.solc_version >= "0.5.0": + return results + for c in self.contracts: + for f in c.functions: + if f.contract_declarer != c: + continue + if f.view or f.pure: + if f.contains_assembly: + attr = 'view' if f.view else 'pure' - info = [f, f' is declared {attr} but contains assembly code\n'] - res = self.generate_result(info, {'contains_assembly': True}) + info = [f, f' is declared {attr} but contains assembly code\n'] + res = self.generate_result(info, {'contains_assembly': True}) - results.append(res) + results.append(res) return results diff --git a/slither/detectors/attributes/const_functions_state.py b/slither/detectors/attributes/const_functions_state.py index 6b39be702..fb7f94c7b 100644 --- a/slither/detectors/attributes/const_functions_state.py +++ b/slither/detectors/attributes/const_functions_state.py @@ -50,24 +50,25 @@ All the calls to `get` revert, breaking Bob's smart contract execution.''' list: {'vuln', 'filename,'contract','func','#varsWritten'} """ results = [] - if self.slither.solc_version < "0.5.0": - for c in self.contracts: - for f in c.functions: - if f.contract_declarer != c: - continue - if f.view or f.pure: - variables_written = f.all_state_variables_written() - if variables_written: - attr = 'view' if f.view else 'pure' + if self.slither.solc_version and self.slither.solc_version >= "0.5.0": + return results + for c in self.contracts: + for f in c.functions: + if f.contract_declarer != c: + continue + if f.view or f.pure: + variables_written = f.all_state_variables_written() + if variables_written: + attr = 'view' if f.view else 'pure' - info = [f, f' is declared {attr} but changes state variables:\n'] + info = [f, f' is declared {attr} but changes state variables:\n'] - for variable_written in variables_written: - info += ['\t- ', variable_written, '\n'] + for variable_written in variables_written: + info += ['\t- ', variable_written, '\n'] - res = self.generate_result(info, {'contains_assembly': False}) + res = self.generate_result(info, {'contains_assembly': False}) - results.append(res) + results.append(res) return results