From 841f460c33c1f9afa5f8e0093631663da15b061d Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 16 Dec 2022 11:02:52 -0600 Subject: [PATCH 01/38] detect local shadowing of named returns vars --- slither/detectors/shadowing/local.py | 10 + .../0.4.25/shadowing_local_variable.sol | 10 + ...al_variable.sol.0.4.25.LocalShadowing.json | 123 ++++++++ .../0.5.16/shadowing_local_variable.sol | 14 + ...al_variable.sol.0.5.16.LocalShadowing.json | 286 ++++++++++++++++++ .../0.6.11/shadowing_local_variable.sol | 14 + ...al_variable.sol.0.6.11.LocalShadowing.json | 286 ++++++++++++++++++ .../0.7.6/shadowing_local_variable.sol | 14 + ...cal_variable.sol.0.7.6.LocalShadowing.json | 282 +++++++++++++++++ 9 files changed, 1039 insertions(+) diff --git a/slither/detectors/shadowing/local.py b/slither/detectors/shadowing/local.py index 617c24be4..2e06886b0 100644 --- a/slither/detectors/shadowing/local.py +++ b/slither/detectors/shadowing/local.py @@ -49,6 +49,7 @@ contract Bug { OVERSHADOWED_MODIFIER = "modifier" OVERSHADOWED_STATE_VARIABLE = "state variable" OVERSHADOWED_EVENT = "event" + OVERSHADOWED_RETURN_VARIABLE = "return variable" def detect_shadowing_definitions(self, contract): # pylint: disable=too-many-branches """Detects if functions, access modifiers, events, state variables, and local variables are named after @@ -86,6 +87,15 @@ contract Bug { overshadowed.append( (self.OVERSHADOWED_STATE_VARIABLE, scope_state_variable) ) + # Check named return variables + for named_return in function.returns: + # Shadowed local delcarations in the same function will have "_scope_" in their name. + # See `FunctionSolc._add_local_variable` + if ( + "_scope_" in variable.name + and variable.name.split("_scope_")[0] == named_return.name + ): + overshadowed.append((self.OVERSHADOWED_RETURN_VARIABLE, named_return)) # If we have found any overshadowed objects, we'll want to add it to our result list. if overshadowed: diff --git a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol index b67833f05..acd93bb46 100644 --- a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol @@ -24,3 +24,13 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function good() external view returns(uint val1) { + return val1; + } +} \ No newline at end of file diff --git a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json index ce0f062a6..e12b5f26d 100644 --- a/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol.0.4.25.LocalShadowing.json @@ -204,6 +204,129 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 533, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 486, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 434, + "length": 225, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 470, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 434, + "length": 225, + "filename_relative": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.4.25/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol index ec17297e6..385f6194c 100644 --- a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol @@ -24,3 +24,17 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function shadowedReturn() external view returns(uint local) { + uint local = 1; + return local; + } + function good() external view returns(uint val1) { + return val1; + } +} \ No newline at end of file diff --git a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json index 1385b0e98..d4d54ad9d 100644 --- a/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol.0.5.16.LocalShadowing.json @@ -204,6 +204,137 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 536, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 489, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 473, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { @@ -567,6 +698,161 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "local_scope_0", + "source_mapping": { + "start": 653, + "length": 14, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 34 + ], + "starting_column": 9, + "ending_column": 23 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 583, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + }, + { + "type": "variable", + "name": "local", + "source_mapping": { + "start": 631, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33 + ], + "starting_column": 53, + "ending_column": 63 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 583, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 437, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedReturn().local_scope_0 (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#34) shadows:\n\t- LocalReturnVariables.shadowedReturn().local (tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#33) (return variable)\n", + "markdown": "[LocalReturnVariables.shadowedReturn().local_scope_0](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L34) shadows:\n\t- [LocalReturnVariables.shadowedReturn().local](tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L33) (return variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.5.16/shadowing_local_variable.sol#L34", + "id": "cd63bdf3f6420e4e109d20ec44b52fcbcbde1c5b6a0701fc6994b35960ab1e85", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol index 450dc851b..c44fe934e 100644 --- a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol @@ -24,3 +24,17 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint __x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function shadowedReturn() external view returns(uint local) { + uint local = 1; + return local; + } + function good() external view returns(uint val1) { + return val1; + } +} \ No newline at end of file diff --git a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json index 0c495f07c..2dc747505 100644 --- a/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol.0.6.11.LocalShadowing.json @@ -1,5 +1,136 @@ [ [ + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 541, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 494, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 478, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { @@ -486,6 +617,161 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "local_scope_0", + "source_mapping": { + "start": 658, + "length": 14, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 34 + ], + "starting_column": 9, + "ending_column": 23 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + }, + { + "type": "variable", + "name": "local", + "source_mapping": { + "start": 636, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33 + ], + "starting_column": 53, + "ending_column": 63 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40, + 41 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "shadowedReturn()" + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedReturn().local_scope_0 (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#34) shadows:\n\t- LocalReturnVariables.shadowedReturn().local (tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#33) (return variable)\n", + "markdown": "[LocalReturnVariables.shadowedReturn().local_scope_0](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L34) shadows:\n\t- [LocalReturnVariables.shadowedReturn().local](tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L33) (return variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.6.11/shadowing_local_variable.sol#L34", + "id": "cd63bdf3f6420e4e109d20ec44b52fcbcbde1c5b6a0701fc6994b35960ab1e85", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { diff --git a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol index 450dc851b..e43477e54 100644 --- a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol +++ b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol @@ -24,3 +24,17 @@ contract FurtherExtendedContract is ExtendedContract { function shadowingParent(uint __x) public pure { int y; uint z; uint w; uint v; } } + +contract LocalReturnVariables { + uint state; + function shadowedState() external view returns(uint state) { + return state; + } + function shadowedReturn() external view returns(uint local) { + uint local = 1; + return local; + } + function good() external view returns(uint val1) { + return val1; + } +} diff --git a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json index 9ea9783e1..e013dc0eb 100644 --- a/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json +++ b/tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol.0.7.6.LocalShadowing.json @@ -1,5 +1,134 @@ [ [ + { + "elements": [ + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 541, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30 + ], + "starting_column": 52, + "ending_column": 62 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedState", + "source_mapping": { + "start": 494, + "length": 88, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 30, + 31, + 32 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "shadowedState()" + } + } + } + }, + { + "type": "variable", + "name": "state", + "source_mapping": { + "start": 478, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 29 + ], + "starting_column": 5, + "ending_column": 15 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedState().state (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#30) shadows:\n\t- LocalReturnVariables.state (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#29) (state variable)\n", + "markdown": "[LocalReturnVariables.shadowedState().state](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L30) shadows:\n\t- [LocalReturnVariables.state](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L29) (state variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L30", + "id": "1b0030affabcff703e57e4f388b86dbda0f412e51ba8d15248bcae9e4748a012", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { @@ -486,6 +615,159 @@ "impact": "Low", "confidence": "High" }, + { + "elements": [ + { + "type": "variable", + "name": "local_scope_0", + "source_mapping": { + "start": 658, + "length": 14, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 34 + ], + "starting_column": 9, + "ending_column": 23 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "shadowedReturn()" + } + } + } + }, + { + "type": "variable", + "name": "local", + "source_mapping": { + "start": 636, + "length": 10, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33 + ], + "starting_column": 53, + "ending_column": 63 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "shadowedReturn", + "source_mapping": { + "start": 588, + "length": 113, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 33, + 34, + 35, + 36 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "LocalReturnVariables", + "source_mapping": { + "start": 442, + "length": 344, + "filename_relative": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol", + "is_dependency": false, + "lines": [ + 28, + 29, + 30, + 31, + 32, + 33, + 34, + 35, + 36, + 37, + 38, + 39, + 40 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "shadowedReturn()" + } + } + } + } + ], + "description": "LocalReturnVariables.shadowedReturn().local_scope_0 (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#34) shadows:\n\t- LocalReturnVariables.shadowedReturn().local (tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#33) (return variable)\n", + "markdown": "[LocalReturnVariables.shadowedReturn().local_scope_0](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L34) shadows:\n\t- [LocalReturnVariables.shadowedReturn().local](tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L33) (return variable)\n", + "first_markdown_element": "tests/detectors/shadowing-local/0.7.6/shadowing_local_variable.sol#L34", + "id": "cd63bdf3f6420e4e109d20ec44b52fcbcbde1c5b6a0701fc6994b35960ab1e85", + "check": "shadowing-local", + "impact": "Low", + "confidence": "High" + }, { "elements": [ { From 47ce2733fc2bb7da4f987d87ef6a9873aa3ef864 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Thu, 2 Feb 2023 12:51:25 -0600 Subject: [PATCH 02/38] Revert "show ignored findings by default for checklist" --- slither/__main__.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/slither/__main__.py b/slither/__main__.py index 9d611532e..d41c8b9d6 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -70,9 +70,6 @@ def process_single( ast = "--ast-compact-json" if args.legacy_ast: ast = "--ast-json" - if args.checklist: - args.show_ignored_findings = True - slither = Slither(target, ast_format=ast, **vars(args)) return _process(slither, detector_classes, printer_classes) From 8bb4ffac62d2d6eed2c76ab503b0b31d21135a86 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Tue, 14 Feb 2023 14:06:54 -0600 Subject: [PATCH 03/38] move assertion to proper branch --- slither/slithir/convert.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 89f85499c..eaec6233e 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -554,9 +554,9 @@ def propagate_types(ir, node: "Node"): # pylint: disable=too-many-locals if (isinstance(t, ElementaryType) and t.name == "address") or ( isinstance(t, TypeAlias) and t.underlying_type.name == "address" ): - # Cannot be a top level function with this. - assert isinstance(node_function, FunctionContract) if ir.destination.name == "this": + # Cannot be a top level function with this. + assert isinstance(node_function, FunctionContract) # the target contract is the contract itself return convert_type_of_high_and_internal_level_call( ir, node_function.contract From 8135a237048085f9e914f8071270e7ca804f3f05 Mon Sep 17 00:00:00 2001 From: Simone Date: Wed, 22 Feb 2023 18:53:22 +0100 Subject: [PATCH 04/38] improve TypeAlias support for echidna --- slither/printers/guidance/echidna.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/slither/printers/guidance/echidna.py b/slither/printers/guidance/echidna.py index 0df47e965..0ee8a0901 100644 --- a/slither/printers/guidance/echidna.py +++ b/slither/printers/guidance/echidna.py @@ -28,6 +28,7 @@ from slither.slithir.operations import ( InternalCall, TypeConversion, ) +from slither.core.solidity_types import TypeAlias, Type from slither.slithir.operations.binary import Binary from slither.slithir.variables import Constant from slither.visitors.expression.constants_folding import ConstantFolding @@ -184,7 +185,11 @@ def _extract_constants_from_irs( # pylint: disable=too-many-branches,too-many-n all_cst_used.append(ConstantValue(str(cst.value), str(type_))) if isinstance(ir, TypeConversion): if isinstance(ir.variable, Constant): - all_cst_used.append(ConstantValue(str(ir.variable.value), str(ir.type))) + if isinstance(ir.type, TypeAlias): + value_type = ir.type.type + else: + value_type = ir.type + all_cst_used.append(ConstantValue(str(ir.variable.value), str(value_type))) continue if ( isinstance(ir, Member) From 56aaa4da4d2c71c199cc43c640754285943bd87a Mon Sep 17 00:00:00 2001 From: Simone Date: Wed, 22 Feb 2023 18:53:48 +0100 Subject: [PATCH 05/38] Fix TypeAlias in solidity signature --- slither/utils/type.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/slither/utils/type.py b/slither/utils/type.py index 5c7fcee34..a6688b13e 100644 --- a/slither/utils/type.py +++ b/slither/utils/type.py @@ -1,7 +1,7 @@ import math from typing import List, Union, Set -from slither.core.solidity_types import ArrayType, MappingType, ElementaryType, UserDefinedType +from slither.core.solidity_types import ArrayType, MappingType, ElementaryType, UserDefinedType, TypeAlias from slither.core.solidity_types.type import Type from slither.core.variables.variable import Variable @@ -88,6 +88,9 @@ def convert_type_for_solidity_signature(t: Type, seen: Set[Type]) -> Union[Type, for x in underlying_type.elems_ordered ] return types + + if isinstance(t, TypeAlias): + return t.type return t From 4474bc2c72977e4bed854cb0e1fd043893c3f5d8 Mon Sep 17 00:00:00 2001 From: bart1e Date: Sun, 26 Feb 2023 18:51:13 +0100 Subject: [PATCH 06/38] WIKI URL fixed --- slither/detectors/functions/cyclomatic_complexity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/detectors/functions/cyclomatic_complexity.py b/slither/detectors/functions/cyclomatic_complexity.py index f03cf61b8..16dc63012 100644 --- a/slither/detectors/functions/cyclomatic_complexity.py +++ b/slither/detectors/functions/cyclomatic_complexity.py @@ -22,7 +22,7 @@ class CyclomaticComplexity(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH - WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity"' + WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity' WIKI_TITLE = "Cyclomatic complexity" WIKI_DESCRIPTION = "Detects functions with high (> 11) cyclomatic complexity." From 5f6551d285313ddea59d7ef3e65ef4e5dff41ad5 Mon Sep 17 00:00:00 2001 From: bart1e Date: Sun, 26 Feb 2023 20:14:38 +0100 Subject: [PATCH 07/38] Black run --- slither/detectors/functions/cyclomatic_complexity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/detectors/functions/cyclomatic_complexity.py b/slither/detectors/functions/cyclomatic_complexity.py index 16dc63012..53212fd4f 100644 --- a/slither/detectors/functions/cyclomatic_complexity.py +++ b/slither/detectors/functions/cyclomatic_complexity.py @@ -22,7 +22,7 @@ class CyclomaticComplexity(AbstractDetector): IMPACT = DetectorClassification.INFORMATIONAL CONFIDENCE = DetectorClassification.HIGH - WIKI = 'https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity' + WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#cyclomatic-complexity" WIKI_TITLE = "Cyclomatic complexity" WIKI_DESCRIPTION = "Detects functions with high (> 11) cyclomatic complexity." From 331cc24d072f242f4b361ce6144dc9d4aed2c939 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Sun, 5 Mar 2023 19:55:17 -0600 Subject: [PATCH 08/38] upgrade nix installation to fix CI installation --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fef698206..50877e262 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,7 +67,7 @@ jobs: - name: Set up nix if: matrix.type == 'dapp' - uses: cachix/install-nix-action@v16 + uses: cachix/install-nix-action@v20 - name: Set up cachix if: matrix.type == 'dapp' From 9e7ff8fd1db397fb7b96f5ae4b28d0935958a9a8 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Tue, 24 Jan 2023 17:35:35 -0600 Subject: [PATCH 09/38] support duplicated func name for diff types in global using for --- slither/slithir/convert.py | 9 +++------ .../declarations/using_for_top_level.py | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index 87a6b075b..f6feefb46 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -525,9 +525,7 @@ def _convert_type_contract(ir: Member) -> Assignment: raise SlithIRError(f"type({contract.name}).{ir.variable_right} is unknown") -def propagate_types( - ir: slither.slithir.operations.operation.Operation, node: "Node" -): # pylint: disable=too-many-locals +def propagate_types(ir: Operation, node: "Node"): # pylint: disable=too-many-locals # propagate the type node_function = node.function using_for = ( @@ -1425,9 +1423,8 @@ def look_for_library_or_top_level( for destination in using_for[t]: if isinstance(destination, FunctionTopLevel) and destination.name == ir.function_name: arguments = [ir.destination] + ir.arguments - if ( - len(destination.parameters) == len(arguments) - and _find_function_from_parameter(arguments, [destination], True) is not None + if len(destination.parameters) == len(arguments) and _find_function_from_parameter( + arguments, [destination], True ): internalcall = InternalCall(destination, ir.nbr_arguments, ir.lvalue, ir.type_call) internalcall.set_expression(ir.expression) diff --git a/slither/solc_parsing/declarations/using_for_top_level.py b/slither/solc_parsing/declarations/using_for_top_level.py index b16fadc40..d0598a734 100644 --- a/slither/solc_parsing/declarations/using_for_top_level.py +++ b/slither/solc_parsing/declarations/using_for_top_level.py @@ -49,7 +49,7 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- type_name = parse_type(self._type_name, self) self._using_for.using_for[type_name] = [] - if self._library_name is not None: + if self._library_name: library_name = parse_type(self._library_name, self) self._using_for.using_for[type_name].append(library_name) self._propagate_global(type_name) @@ -91,7 +91,12 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- self, function_name: str, type_name: Union[TypeAliasTopLevel, UserDefinedType] ) -> None: for tl_function in self.compilation_unit.functions_top_level: - if tl_function.name == function_name: + # The library function is bound to the first parameter's type + if ( + tl_function.name == function_name + and tl_function.parameters + and type_name == tl_function.parameters[0].type + ): self._using_for.using_for[type_name].append(tl_function) self._propagate_global(type_name) break @@ -108,7 +113,12 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- break if c.name == library_name: for cf in c.functions: - if cf.name == function_name: + # The library function is bound to the first parameter's type + if ( + cf.name == function_name + and cf.parameters + and type_name == cf.parameters[0].type + ): self._using_for.using_for[type_name].append(cf) self._propagate_global(type_name) found = True From fdd42329a62e15c935e6bad4a320b97782ecdf51 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 25 Jan 2023 09:23:23 -0600 Subject: [PATCH 10/38] only look for library functions in the current scope --- slither/slithir/convert.py | 5 +++-- slither/solc_parsing/declarations/using_for_top_level.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py index f6feefb46..8faaf8775 100644 --- a/slither/slithir/convert.py +++ b/slither/slithir/convert.py @@ -1423,8 +1423,9 @@ def look_for_library_or_top_level( for destination in using_for[t]: if isinstance(destination, FunctionTopLevel) and destination.name == ir.function_name: arguments = [ir.destination] + ir.arguments - if len(destination.parameters) == len(arguments) and _find_function_from_parameter( - arguments, [destination], True + if ( + len(destination.parameters) == len(arguments) + and _find_function_from_parameter(arguments, [destination], True) is not None ): internalcall = InternalCall(destination, ir.nbr_arguments, ir.lvalue, ir.type_call) internalcall.set_expression(ir.expression) diff --git a/slither/solc_parsing/declarations/using_for_top_level.py b/slither/solc_parsing/declarations/using_for_top_level.py index d0598a734..707ad83ac 100644 --- a/slither/solc_parsing/declarations/using_for_top_level.py +++ b/slither/solc_parsing/declarations/using_for_top_level.py @@ -90,7 +90,7 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- def _analyze_top_level_function( self, function_name: str, type_name: Union[TypeAliasTopLevel, UserDefinedType] ) -> None: - for tl_function in self.compilation_unit.functions_top_level: + for tl_function in self._using_for.file_scope.functions: # The library function is bound to the first parameter's type if ( tl_function.name == function_name From 166a8cefe239a60d03a7e8c0dde4ffef835fd49e Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Feb 2023 22:25:35 -0600 Subject: [PATCH 11/38] add test files --- tests/using-for-global-collision/README.md | 3 +++ tests/using-for-global-collision/foundry.toml | 6 ++++++ tests/using-for-global-collision/src/MyTypeA.sol | 2 ++ tests/using-for-global-collision/src/MyTypeA/Casting.sol | 4 ++++ tests/using-for-global-collision/src/MyTypeA/Math.sol | 9 +++++++++ tests/using-for-global-collision/src/MyTypeA/Type.sol | 5 +++++ tests/using-for-global-collision/src/MyTypeB.sol | 2 ++ tests/using-for-global-collision/src/MyTypeB/Casting.sol | 4 ++++ tests/using-for-global-collision/src/MyTypeB/Math.sol | 6 ++++++ tests/using-for-global-collision/src/MyTypeB/Type.sol | 6 ++++++ tests/using-for-global-collision/src/Test.sol | 7 +++++++ 11 files changed, 54 insertions(+) create mode 100644 tests/using-for-global-collision/README.md create mode 100644 tests/using-for-global-collision/foundry.toml create mode 100644 tests/using-for-global-collision/src/MyTypeA.sol create mode 100644 tests/using-for-global-collision/src/MyTypeA/Casting.sol create mode 100644 tests/using-for-global-collision/src/MyTypeA/Math.sol create mode 100644 tests/using-for-global-collision/src/MyTypeA/Type.sol create mode 100644 tests/using-for-global-collision/src/MyTypeB.sol create mode 100644 tests/using-for-global-collision/src/MyTypeB/Casting.sol create mode 100644 tests/using-for-global-collision/src/MyTypeB/Math.sol create mode 100644 tests/using-for-global-collision/src/MyTypeB/Type.sol create mode 100644 tests/using-for-global-collision/src/Test.sol diff --git a/tests/using-for-global-collision/README.md b/tests/using-for-global-collision/README.md new file mode 100644 index 000000000..9cf3657e0 --- /dev/null +++ b/tests/using-for-global-collision/README.md @@ -0,0 +1,3 @@ +# README + +Before using this project, run `forge init --no-git --no-commit --force` to initialize submodules diff --git a/tests/using-for-global-collision/foundry.toml b/tests/using-for-global-collision/foundry.toml new file mode 100644 index 000000000..e6810b2b5 --- /dev/null +++ b/tests/using-for-global-collision/foundry.toml @@ -0,0 +1,6 @@ +[profile.default] +src = 'src' +out = 'out' +libs = ['lib'] + +# See more config options https://github.com/foundry-rs/foundry/tree/master/config \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA.sol b/tests/using-for-global-collision/src/MyTypeA.sol new file mode 100644 index 000000000..dbb00faf2 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA.sol @@ -0,0 +1,2 @@ +import "./MyTypeA/Type.sol"; +import "./MyTypeA/Math.sol"; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA/Casting.sol b/tests/using-for-global-collision/src/MyTypeA/Casting.sol new file mode 100644 index 000000000..c166436fe --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA/Casting.sol @@ -0,0 +1,4 @@ +import "./Type.sol"; +function unwrap(MyTypeA a) pure returns (int256) { + return MyTypeA.unwrap(a); +} \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA/Math.sol b/tests/using-for-global-collision/src/MyTypeA/Math.sol new file mode 100644 index 000000000..328ddc64a --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA/Math.sol @@ -0,0 +1,9 @@ +import "./Type.sol"; + +function mul(MyTypeA a, MyTypeA b) pure returns (MyTypeA) { + return MyTypeA.wrap(MyTypeA.unwrap(a) * MyTypeA.unwrap(b)); +} + +function unwrap(MyTypeA a) pure returns (int256) { + return MyTypeA.unwrap(a); +} \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA/Type.sol b/tests/using-for-global-collision/src/MyTypeA/Type.sol new file mode 100644 index 000000000..ee606f3d4 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeA/Type.sol @@ -0,0 +1,5 @@ +import "./Math.sol" as m; + +type MyTypeA is int256; + +using {m.mul, m.unwrap} for MyTypeA global; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeB.sol b/tests/using-for-global-collision/src/MyTypeB.sol new file mode 100644 index 000000000..3ddde2ac6 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB.sol @@ -0,0 +1,2 @@ +import "./MyTypeB/Type.sol"; +import "./MyTypeB/Math.sol"; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeB/Casting.sol b/tests/using-for-global-collision/src/MyTypeB/Casting.sol new file mode 100644 index 000000000..c400a9112 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB/Casting.sol @@ -0,0 +1,4 @@ +import "./Type.sol"; +function unwrap(MyTypeB a) pure returns (uint256) { + return MyTypeB.unwrap(a); +} \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeB/Math.sol b/tests/using-for-global-collision/src/MyTypeB/Math.sol new file mode 100644 index 000000000..24ee1a582 --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB/Math.sol @@ -0,0 +1,6 @@ +import "./Type.sol"; + +function mul(MyTypeB a, MyTypeB b) pure returns (MyTypeB) { + return MyTypeB.wrap(MyTypeB.unwrap(a) * MyTypeB.unwrap(b)); +} + diff --git a/tests/using-for-global-collision/src/MyTypeB/Type.sol b/tests/using-for-global-collision/src/MyTypeB/Type.sol new file mode 100644 index 000000000..a66b65f5d --- /dev/null +++ b/tests/using-for-global-collision/src/MyTypeB/Type.sol @@ -0,0 +1,6 @@ +import "./Casting.sol" as C; +import "./Math.sol" as M; + +type MyTypeB is uint256; + +using {M.mul, C.unwrap} for MyTypeB global; \ No newline at end of file diff --git a/tests/using-for-global-collision/src/Test.sol b/tests/using-for-global-collision/src/Test.sol new file mode 100644 index 000000000..013570048 --- /dev/null +++ b/tests/using-for-global-collision/src/Test.sol @@ -0,0 +1,7 @@ +import "./MyTypeB.sol"; + +contract UsingForGlobalTopLevelCollision { + function mulAndUnwrap(MyTypeB x, MyTypeB y) external pure returns (uint256 z) { + z = x.mul(y).unwrap(); + } +} \ No newline at end of file From 3cc55102c7f91b92ec5ace3b8756fceb60251303 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Feb 2023 22:28:48 -0600 Subject: [PATCH 12/38] fix import --- tests/using-for-global-collision/src/MyTypeA/Math.sol | 4 ---- tests/using-for-global-collision/src/MyTypeA/Type.sol | 5 +++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/using-for-global-collision/src/MyTypeA/Math.sol b/tests/using-for-global-collision/src/MyTypeA/Math.sol index 328ddc64a..21f7c7925 100644 --- a/tests/using-for-global-collision/src/MyTypeA/Math.sol +++ b/tests/using-for-global-collision/src/MyTypeA/Math.sol @@ -3,7 +3,3 @@ import "./Type.sol"; function mul(MyTypeA a, MyTypeA b) pure returns (MyTypeA) { return MyTypeA.wrap(MyTypeA.unwrap(a) * MyTypeA.unwrap(b)); } - -function unwrap(MyTypeA a) pure returns (int256) { - return MyTypeA.unwrap(a); -} \ No newline at end of file diff --git a/tests/using-for-global-collision/src/MyTypeA/Type.sol b/tests/using-for-global-collision/src/MyTypeA/Type.sol index ee606f3d4..0973c7869 100644 --- a/tests/using-for-global-collision/src/MyTypeA/Type.sol +++ b/tests/using-for-global-collision/src/MyTypeA/Type.sol @@ -1,5 +1,6 @@ -import "./Math.sol" as m; +import "./Casting.sol" as C; +import "./Math.sol" as M; type MyTypeA is int256; -using {m.mul, m.unwrap} for MyTypeA global; \ No newline at end of file +using {M.mul, C.unwrap} for MyTypeA global; \ No newline at end of file From 17510915fe5be2ace720b562baab19191db8ca56 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Mon, 6 Mar 2023 10:35:02 -0600 Subject: [PATCH 13/38] add test --- tests/conftest.py | 58 ++++++++++++++++++++++++++++++++++++++++++ tests/test_features.py | 5 ++++ 2 files changed, 63 insertions(+) create mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 000000000..ca1dcf8ea --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,58 @@ +import os +from pathlib import Path +from contextlib import contextmanager +from typing import Optional + +import pytest +from slither import Slither +from solc_select import solc_select +from crytic_compile import CryticCompile +from crytic_compile.platform.solc_standard_json import SolcStandardJson + + +@contextmanager +def _select_solc_version(version: Optional[str]): + """Selects solc version to use for running tests. + If no version is provided, latest is used.""" + if not version: + # This sorts the versions numerically + vers = sorted( + map( + lambda x: (int(x[0]), int(x[1]), int(x[2])), + map(lambda x: x.split(".", 3), solc_select.installed_versions()), + ) + ) + ver = list(vers)[-1] + version = ".".join(map(str, ver)) + env = dict(os.environ) + env_restore = dict(env) + env["SOLC_VERSION"] = version + os.environ.clear() + os.environ.update(env) + + yield version + + os.environ.clear() + os.environ.update(env_restore) + + +@pytest.fixture(name="select_solc_version") +def fixture_select_solc_version(): + return _select_solc_version + + +@pytest.fixture +def slither_from_dir(select_solc_version): + @contextmanager + def _slither_from_dir(directory: str, solc_version: Optional[str] = None): + """Yields a Slither instance using solidity files in directory and solc_version. + Temporarily changes the solc-version temporary to solc_version. + """ + standard_json = SolcStandardJson() + for source_file in Path(directory).rglob("*.sol"): + standard_json.add_source_file(Path(source_file).as_posix()) + with select_solc_version(solc_version): + compilation = CryticCompile(standard_json) + yield Slither(compilation) + + return _slither_from_dir diff --git a/tests/test_features.py b/tests/test_features.py index d29a5eb6a..247d955a4 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -1,4 +1,5 @@ import inspect +from tests.conftest import slither_from_dir from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson @@ -160,3 +161,7 @@ def test_arithmetic_usage() -> None: assert { f.source_mapping.content_hash for f in unchecked_arithemtic_usage(slither.contracts[0]) } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"} + +def test_using_for_global_collision(slither_from_dir): + with slither_from_dir("./tests/using-for-global-collision") as sl: + _run_all_detectors(sl) From 3c0ef749324a10a6f2743bf688a7be0e1493ec7d Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Mon, 6 Mar 2023 10:52:10 -0600 Subject: [PATCH 14/38] fmt --- tests/test_features.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_features.py b/tests/test_features.py index 247d955a4..0fafb6daf 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -161,7 +161,8 @@ def test_arithmetic_usage() -> None: assert { f.source_mapping.content_hash for f in unchecked_arithemtic_usage(slither.contracts[0]) } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"} - + + def test_using_for_global_collision(slither_from_dir): with slither_from_dir("./tests/using-for-global-collision") as sl: _run_all_detectors(sl) From 1616f93364253bf8009618f742ed824424808c4b Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Mon, 6 Mar 2023 10:53:33 -0600 Subject: [PATCH 15/38] rm misc files --- tests/using-for-global-collision/README.md | 3 --- tests/using-for-global-collision/foundry.toml | 6 ------ 2 files changed, 9 deletions(-) delete mode 100644 tests/using-for-global-collision/README.md delete mode 100644 tests/using-for-global-collision/foundry.toml diff --git a/tests/using-for-global-collision/README.md b/tests/using-for-global-collision/README.md deleted file mode 100644 index 9cf3657e0..000000000 --- a/tests/using-for-global-collision/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# README - -Before using this project, run `forge init --no-git --no-commit --force` to initialize submodules diff --git a/tests/using-for-global-collision/foundry.toml b/tests/using-for-global-collision/foundry.toml deleted file mode 100644 index e6810b2b5..000000000 --- a/tests/using-for-global-collision/foundry.toml +++ /dev/null @@ -1,6 +0,0 @@ -[profile.default] -src = 'src' -out = 'out' -libs = ['lib'] - -# See more config options https://github.com/foundry-rs/foundry/tree/master/config \ No newline at end of file From 98d1c5043773f4387137cdc13401ee033c776241 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Tue, 7 Mar 2023 11:34:59 -0600 Subject: [PATCH 16/38] lint --- tests/conftest.py | 7 ++++--- tests/test_features.py | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index ca1dcf8ea..85567af98 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,14 @@ import os -from pathlib import Path from contextlib import contextmanager +from pathlib import Path from typing import Optional import pytest -from slither import Slither -from solc_select import solc_select from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson +from solc_select import solc_select + +from slither import Slither @contextmanager diff --git a/tests/test_features.py b/tests/test_features.py index 0fafb6daf..b2a3960af 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -1,5 +1,4 @@ import inspect -from tests.conftest import slither_from_dir from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson @@ -9,7 +8,7 @@ from slither import Slither from slither.core.variables.state_variable import StateVariable from slither.detectors import all_detectors from slither.detectors.abstract_detector import AbstractDetector -from slither.slithir.operations import LibraryCall, InternalCall +from slither.slithir.operations import InternalCall, LibraryCall from slither.utils.arithmetic import unchecked_arithemtic_usage @@ -163,6 +162,6 @@ def test_arithmetic_usage() -> None: } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"} -def test_using_for_global_collision(slither_from_dir): +def test_using_for_global_collision(slither_from_dir) -> None: with slither_from_dir("./tests/using-for-global-collision") as sl: _run_all_detectors(sl) From 36052fdc4eae73b89a48b8c9c0ba70cef81cb33a Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Wed, 8 Mar 2023 11:09:16 +0100 Subject: [PATCH 17/38] Sync cc@master Use cc@master Remove rinkeby test (etherscan does not support rinkeby anymore) --- scripts/ci_test_etherscan.sh | 7 ------- setup.py | 4 ++-- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/scripts/ci_test_etherscan.sh b/scripts/ci_test_etherscan.sh index 694690691..366b0283d 100755 --- a/scripts/ci_test_etherscan.sh +++ b/scripts/ci_test_etherscan.sh @@ -17,12 +17,5 @@ if [ "$GITHUB_ETHERSCAN" = "" ]; then sleep $(( ( RANDOM % 5 ) + 1 ))s fi -echo "::group::Etherscan rinkeby" -if ! slither rinkeby:0xFe05820C5A92D9bc906D4A46F662dbeba794d3b7 --etherscan-apikey "$GITHUB_ETHERSCAN" --no-fail-pedantic; then - echo "Etherscan rinkeby test failed" - exit 1 -fi -echo "::endgroup::" - exit 0 diff --git a/setup.py b/setup.py index 7879ebf46..cb2ed6482 100644 --- a/setup.py +++ b/setup.py @@ -15,8 +15,8 @@ setup( "packaging", "prettytable>=0.7.2", "pycryptodome>=3.4.6", - "crytic-compile>=0.3.0", - # "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile", + # "crytic-compile>=0.3.0", + "crytic-compile@git+https://github.com/crytic/crytic-compile.git@master#egg=crytic-compile", ], extras_require={ "dev": [ From a42c2cc0d1b8a4e4b34e55fb9166fe1cb3a48a15 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Wed, 8 Mar 2023 15:49:23 +0530 Subject: [PATCH 18/38] deregister_detector This function would be helpful to de-register a detector class from the list of detectors already registered with the slither object --- slither/slither.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/slither/slither.py b/slither/slither.py index 3e44944b3..1be371902 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -187,6 +187,15 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes for compilation_unit in self.compilation_units: instance = detector_class(compilation_unit, self, logger_detector) self._detectors.append(instance) + + def deregister_detector(self, detector_class: Type[AbstractDetector]) -> None: + """ + :param detector_class: Class inheriting from `AbstractDetector`. + """ + + for obj in self._detectors: + if type(obj) == detector_class : + self._detectors.remove(obj) def register_printer(self, printer_class: Type[AbstractPrinter]) -> None: """ From 4e2f1ac3cc9858ebc1fbeaa538881a055acc64ec Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:03:45 +0530 Subject: [PATCH 19/38] deregister_printer This function would be helpful to de-register a printer class from the list of _printers already registered with the slither object. --- slither/slither.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index 3e44944b3..ef995adce 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -196,7 +196,17 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes instance = printer_class(self, logger_printer) self._printers.append(instance) - + + def deregister_printer(self, printer_class: Type[AbstractPrinter]) -> None: + """ + :param printer_class: Class inheriting from `AbstractPrinter`. + """ + + for obj in self._printers: + if type(obj) == printer_class : + self._printers.remove(obj) + return + def run_detectors(self) -> List[Dict]: """ :return: List of registered detectors results. From 611798c634ad078d9aec5b16fdb66acb081fb075 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:10:57 +0530 Subject: [PATCH 20/38] update deregister_detector() return on removing the first hit itself, as there can't be multiple objects of the same detector class in the _detectors. --- slither/slither.py | 1 + 1 file changed, 1 insertion(+) diff --git a/slither/slither.py b/slither/slither.py index 1be371902..98e173c20 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -196,6 +196,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes for obj in self._detectors: if type(obj) == detector_class : self._detectors.remove(obj) + return def register_printer(self, printer_class: Type[AbstractPrinter]) -> None: """ From 181ee42e71e6d67d8c3d60b8560e06b8ad604325 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:21:10 +0530 Subject: [PATCH 21/38] pylint: disable unidiomatic-typecheck --- slither/slither.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index ef995adce..5453c186d 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -203,7 +203,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes """ for obj in self._printers: - if type(obj) == printer_class : + if type(obj) == printer_class : # pylint: disable=unidiomatic-typecheck self._printers.remove(obj) return From 646fb1e80c4f6b65c289a8a7af71fc3490f82fe7 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Wed, 8 Mar 2023 16:23:12 +0530 Subject: [PATCH 22/38] pylint: disable unidiomatic-typecheck --- slither/slither.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index 98e173c20..2fc078905 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -194,7 +194,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes """ for obj in self._detectors: - if type(obj) == detector_class : + if type(obj) == detector_class : # pylint: disable=unidiomatic-typecheck self._detectors.remove(obj) return From 9acddc3259fe7db22b79e9b9b557633793d58cad Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Mar 2023 08:58:20 -0600 Subject: [PATCH 23/38] include return variables in uninit ptr detector --- .../uninitialized_storage_variables.py | 3 +- .../0.8.19/uninitialized_storage_pointer.sol | 12 +++ ...r.sol.0.8.19.UninitializedStorageVars.json | 85 +++++++++++++++++++ tests/test_detectors.py | 5 ++ 4 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol create mode 100644 tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json diff --git a/slither/detectors/variables/uninitialized_storage_variables.py b/slither/detectors/variables/uninitialized_storage_variables.py index 422996646..b1a0d956b 100644 --- a/slither/detectors/variables/uninitialized_storage_variables.py +++ b/slither/detectors/variables/uninitialized_storage_variables.py @@ -104,8 +104,9 @@ Bob calls `func`. As a result, `owner` is overridden to `0`. for function in contract.functions: if function.is_implemented and function.entry_point: uninitialized_storage_variables = [ - v for v in function.local_variables if v.is_storage and v.uninitialized + v for v in function.variables if v.is_storage and v.uninitialized ] + function.entry_point.context[self.key] = uninitialized_storage_variables self._detect_uninitialized(function, function.entry_point, []) diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol new file mode 100644 index 000000000..cac727ef5 --- /dev/null +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol @@ -0,0 +1,12 @@ +contract Uninitialized{ + + struct St{ + uint a; + } + + function test() internal returns (St storage ret){ + ret = ret; + ret.a += 1; + } + +} diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json new file mode 100644 index 000000000..9c32fe16e --- /dev/null +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json @@ -0,0 +1,85 @@ +[ + [ + { + "elements": [ + { + "type": "variable", + "name": "ret", + "source_mapping": { + "start": 101, + "length": 14, + "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "is_dependency": false, + "lines": [ + 7 + ], + "starting_column": 39, + "ending_column": 53 + }, + "type_specific_fields": { + "parent": { + "type": "function", + "name": "test", + "source_mapping": { + "start": 67, + "length": 96, + "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "is_dependency": false, + "lines": [ + 7, + 8, + 9, + 10 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Uninitialized", + "source_mapping": { + "start": 0, + "length": 170, + "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", + "is_dependency": false, + "lines": [ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12 + ], + "starting_column": 1, + "ending_column": 2 + } + }, + "signature": "test()" + } + } + } + } + ], + "description": "Uninitialized.test().ret (tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#7) is a storage variable never initialized\n", + "markdown": "[Uninitialized.test().ret](tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7) is a storage variable never initialized\n", + "first_markdown_element": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7", + "id": "cc7efd7ba9d430d21b17f18d15e561d2e188b59a3d62b9c5e76eeec765626cbd", + "check": "uninitialized-storage", + "impact": "High", + "confidence": "High" + } + ] +] \ No newline at end of file diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 3d3cc7474..9d82afd2c 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -371,6 +371,11 @@ ALL_TEST_OBJECTS = [ "uninitialized_storage_pointer.sol", "0.4.25", ), + Test( + all_detectors.UninitializedStorageVars, + "uninitialized_storage_pointer.sol", + "0.8.19", + ), Test(all_detectors.TxOrigin, "tx_origin.sol", "0.4.25"), Test(all_detectors.TxOrigin, "tx_origin.sol", "0.5.16"), Test(all_detectors.TxOrigin, "tx_origin.sol", "0.6.11"), From e26a520766441c9a22ca99b2985ffe4fefa9578f Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Mar 2023 10:45:45 -0600 Subject: [PATCH 24/38] exclude storage params --- .../uninitialized_storage_variables.py | 3 ++- .../0.8.19/uninitialized_storage_pointer.sol | 7 ++++- ...r.sol.0.8.19.UninitializedStorageVars.json | 27 +++++++++++-------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/slither/detectors/variables/uninitialized_storage_variables.py b/slither/detectors/variables/uninitialized_storage_variables.py index b1a0d956b..9caa5b88f 100644 --- a/slither/detectors/variables/uninitialized_storage_variables.py +++ b/slither/detectors/variables/uninitialized_storage_variables.py @@ -103,8 +103,9 @@ Bob calls `func`. As a result, `owner` is overridden to `0`. for contract in self.compilation_unit.contracts: for function in contract.functions: if function.is_implemented and function.entry_point: + locals_except_params = set(function.variables) - set(function.parameters) uninitialized_storage_variables = [ - v for v in function.variables if v.is_storage and v.uninitialized + v for v in locals_except_params if v.is_storage and v.uninitialized ] function.entry_point.context[self.key] = uninitialized_storage_variables diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol index cac727ef5..a2fb6a77c 100644 --- a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol @@ -4,9 +4,14 @@ contract Uninitialized{ uint a; } - function test() internal returns (St storage ret){ + function bad() internal returns (St storage ret){ ret = ret; ret.a += 1; } + function ok(St storage ret) internal { + ret = ret; + ret.a += 1; + } + } diff --git a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json index 9c32fe16e..e7fab681d 100644 --- a/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json +++ b/tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol.0.8.19.UninitializedStorageVars.json @@ -6,7 +6,7 @@ "type": "variable", "name": "ret", "source_mapping": { - "start": 101, + "start": 100, "length": 14, "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", "filename_absolute": "/GENERIC_PATH", @@ -15,16 +15,16 @@ "lines": [ 7 ], - "starting_column": 39, - "ending_column": 53 + "starting_column": 38, + "ending_column": 52 }, "type_specific_fields": { "parent": { "type": "function", - "name": "test", + "name": "bad", "source_mapping": { "start": 67, - "length": 96, + "length": 95, "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", @@ -44,7 +44,7 @@ "name": "Uninitialized", "source_mapping": { "start": 0, - "length": 170, + "length": 262, "filename_relative": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", "filename_absolute": "/GENERIC_PATH", "filename_short": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol", @@ -61,22 +61,27 @@ 9, 10, 11, - 12 + 12, + 13, + 14, + 15, + 16, + 17 ], "starting_column": 1, "ending_column": 2 } }, - "signature": "test()" + "signature": "bad()" } } } } ], - "description": "Uninitialized.test().ret (tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#7) is a storage variable never initialized\n", - "markdown": "[Uninitialized.test().ret](tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7) is a storage variable never initialized\n", + "description": "Uninitialized.bad().ret (tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#7) is a storage variable never initialized\n", + "markdown": "[Uninitialized.bad().ret](tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7) is a storage variable never initialized\n", "first_markdown_element": "tests/detectors/uninitialized-storage/0.8.19/uninitialized_storage_pointer.sol#L7", - "id": "cc7efd7ba9d430d21b17f18d15e561d2e188b59a3d62b9c5e76eeec765626cbd", + "id": "979d28e501693ed7ece0d429e7c30266f8e9d6a2e2eedc87006c4bad63e78706", "check": "uninitialized-storage", "impact": "High", "confidence": "High" From 09a9b907afd22c22f92db3ecc046a2565e0dbb4a Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Wed, 8 Mar 2023 23:00:35 -0600 Subject: [PATCH 25/38] restrict variable-scope detector to only solc 0.4.x close https://github.com/crytic/slither/issues/1424 --- .../detectors/variables/predeclaration_usage_local.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/slither/detectors/variables/predeclaration_usage_local.py b/slither/detectors/variables/predeclaration_usage_local.py index 2ba539a91..2a24eed84 100644 --- a/slither/detectors/variables/predeclaration_usage_local.py +++ b/slither/detectors/variables/predeclaration_usage_local.py @@ -7,7 +7,11 @@ from slither.core.cfg.node import Node from slither.core.declarations import Function from slither.core.declarations.contract import Contract from slither.core.variables.local_variable import LocalVariable -from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification +from slither.detectors.abstract_detector import ( + AbstractDetector, + DetectorClassification, + ALL_SOLC_VERSIONS_04, +) from slither.utils.output import Output @@ -53,7 +57,9 @@ Additionally, the for-loop uses the variable `max`, which is declared in a previ # endregion wiki_exploit_scenario WIKI_RECOMMENDATION = "Move all variable declarations prior to any usage of the variable, and ensure that reaching a variable declaration does not depend on some conditional if it is used unconditionally." - + + VULNERABLE_SOLC_VERSIONS = ALL_SOLC_VERSIONS_04 + def detect_predeclared_local_usage( self, node: Node, From e48351339c120f7e339ef5cc9e79d3314140aab0 Mon Sep 17 00:00:00 2001 From: sidarth16 Date: Thu, 9 Mar 2023 12:01:49 +0530 Subject: [PATCH 26/38] black --- slither/slither.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/slither/slither.py b/slither/slither.py index 5453c186d..eebec65aa 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -196,17 +196,17 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes instance = printer_class(self, logger_printer) self._printers.append(instance) - + def deregister_printer(self, printer_class: Type[AbstractPrinter]) -> None: """ :param printer_class: Class inheriting from `AbstractPrinter`. """ - + for obj in self._printers: - if type(obj) == printer_class : # pylint: disable=unidiomatic-typecheck + if type(obj) == printer_class: # pylint: disable=unidiomatic-typecheck self._printers.remove(obj) return - + def run_detectors(self) -> List[Dict]: """ :return: List of registered detectors results. From d5a84364b953d199b3dd2de178b9b0a43764865b Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Thu, 9 Mar 2023 12:06:13 +0530 Subject: [PATCH 27/38] black --- slither/slither.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/slither.py b/slither/slither.py index 2fc078905..2737450b6 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -187,14 +187,14 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes for compilation_unit in self.compilation_units: instance = detector_class(compilation_unit, self, logger_detector) self._detectors.append(instance) - + def deregister_detector(self, detector_class: Type[AbstractDetector]) -> None: """ :param detector_class: Class inheriting from `AbstractDetector`. """ for obj in self._detectors: - if type(obj) == detector_class : # pylint: disable=unidiomatic-typecheck + if type(obj) == detector_class: # pylint: disable=unidiomatic-typecheck self._detectors.remove(obj) return From 76d8321401a9a30a635359d9affbd64f69b524d3 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Thu, 9 Mar 2023 14:07:21 +0100 Subject: [PATCH 28/38] Improve parsing of contract's comment --- slither/core/declarations/contract.py | 27 +++++++++++++++++++ slither/solc_parsing/declarations/contract.py | 27 +++++++++++++++++-- tests/custom_comments/contract_comment.sol | 7 +++++ tests/test_features.py | 24 ++++++++++++++++- 4 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 tests/custom_comments/contract_comment.sol diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 2d2d10b04..95b05aa6b 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -110,6 +110,8 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods Dict["StateVariable", Set[Union["StateVariable", "Function"]]] ] = None + self._comments: Optional[str] = None + ################################################################################### ################################################################################### # region General's properties @@ -165,6 +167,31 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods def is_library(self, is_library: bool): self._is_library = is_library + @property + def comments(self) -> Optional[str]: + """ + Return the comments associated with the contract. + + When using comments, avoid strict text matching, as the solc behavior might change. + For example, for old solc version, the first space after the * is not kept, i.e: + + * @title Test Contract + * @dev Test comment + + Returns + - " @title Test Contract\n @dev Test comment" for newest versions + - "@title Test Contract\n@dev Test comment" for older versions + + + Returns: + the comment as a string + """ + return self._comments + + @comments.setter + def comments(self, comments: str): + self._comments = comments + # endregion ################################################################################### ################################################################################### diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 475c3fab2..e63dbe68f 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -780,12 +780,35 @@ class ContractSolc(CallerContextExpression): self._customErrorParsed = [] def _handle_comment(self, attributes: Dict) -> None: + """ + Save the contract comment in self.comments + And handle custom slither comments + + Args: + attributes: + + Returns: + + """ + # Old solc versions store the comment in attributes["documentation"] + # More recent ones store it in attributes["documentation"]["text"] if ( "documentation" in attributes and attributes["documentation"] is not None - and "text" in attributes["documentation"] + and ( + "text" in attributes["documentation"] + or isinstance(attributes["documentation"], str) + ) ): - candidates = attributes["documentation"]["text"].replace("\n", ",").split(",") + text = ( + attributes["documentation"] + if isinstance(attributes["documentation"], str) + else attributes["documentation"]["text"] + ) + self._contract.comments = text + + # Look for custom comments + candidates = text.replace("\n", ",").split(",") for candidate in candidates: if "@custom:security isDelegatecallProxy" in candidate: diff --git a/tests/custom_comments/contract_comment.sol b/tests/custom_comments/contract_comment.sol new file mode 100644 index 000000000..8f0fb5233 --- /dev/null +++ b/tests/custom_comments/contract_comment.sol @@ -0,0 +1,7 @@ +/** + * @title Test Contract + * @dev Test comment + */ +contract A{ + +} \ No newline at end of file diff --git a/tests/test_features.py b/tests/test_features.py index d29a5eb6a..b42b60616 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -31,7 +31,7 @@ def test_node() -> None: def test_collision() -> None: - + solc_select.switch_global_version("0.8.0", always_install=True) standard_json = SolcStandardJson() standard_json.add_source_file("./tests/collisions/a.sol") standard_json.add_source_file("./tests/collisions/b.sol") @@ -43,6 +43,7 @@ def test_collision() -> None: def test_cycle() -> None: + solc_select.switch_global_version("0.8.0", always_install=True) slither = Slither("./tests/test_cyclic_import/a.sol") _run_all_detectors(slither) @@ -74,6 +75,27 @@ def test_upgradeable_comments() -> None: assert v1.upgradeable_version == "version_1" +def test_contract_comments() -> None: + comments = " @title Test Contract\n @dev Test comment" + + solc_select.switch_global_version("0.8.10", always_install=True) + slither = Slither("./tests/custom_comments/contract_comment.sol") + compilation_unit = slither.compilation_units[0] + contract = compilation_unit.get_contract_from_name("A")[0] + + assert contract.comments == comments + + # Old solc versions have a different parsing of comments + # the initial space (after *) is also not kept on every line + comments = "@title Test Contract\n@dev Test comment" + solc_select.switch_global_version("0.5.16", always_install=True) + slither = Slither("./tests/custom_comments/contract_comment.sol") + compilation_unit = slither.compilation_units[0] + contract = compilation_unit.get_contract_from_name("A")[0] + + assert contract.comments == comments + + def test_using_for_top_level_same_name() -> None: solc_select.switch_global_version("0.8.15", always_install=True) slither = Slither("./tests/ast-parsing/using-for-3-0.8.0.sol") From 3850cf57e3e24bab2a9850463f24de51ed5a9823 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Thu, 9 Mar 2023 20:30:02 +0530 Subject: [PATCH 29/38] renaming to unregister_detector() Co-authored-by: alpharush <0xalpharush@protonmail.com> --- slither/slither.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index 2737450b6..6268c5505 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -188,7 +188,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes instance = detector_class(compilation_unit, self, logger_detector) self._detectors.append(instance) - def deregister_detector(self, detector_class: Type[AbstractDetector]) -> None: + def unregister_detector(self, detector_class: Type[AbstractDetector]) -> None: """ :param detector_class: Class inheriting from `AbstractDetector`. """ From 275be58a55dae59a936d2d7a33bf0a85d6900fc4 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Fri, 10 Mar 2023 08:54:35 +0530 Subject: [PATCH 30/38] isinstance() --- slither/slither.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index 6268c5505..6f1bf61cd 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -194,7 +194,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes """ for obj in self._detectors: - if type(obj) == detector_class: # pylint: disable=unidiomatic-typecheck + if isinstance(obj, detector_class): self._detectors.remove(obj) return From 9b69da1cc84d2f51a33656414251dd521de6ba01 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Fri, 10 Mar 2023 08:59:01 +0530 Subject: [PATCH 31/38] isinstance() --- slither/slither.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index eebec65aa..795f8794b 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -203,7 +203,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes """ for obj in self._printers: - if type(obj) == printer_class: # pylint: disable=unidiomatic-typecheck + if isinstance(obj, printer_class): self._printers.remove(obj) return From 660ccb9751588f143f613bbe17ef4070d5a77d33 Mon Sep 17 00:00:00 2001 From: "S.Sidarth" <38394431+sidarth16@users.noreply.github.com> Date: Fri, 10 Mar 2023 08:59:40 +0530 Subject: [PATCH 32/38] rename to unregister_printer() --- slither/slither.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/slither.py b/slither/slither.py index 795f8794b..42f0346be 100644 --- a/slither/slither.py +++ b/slither/slither.py @@ -197,7 +197,7 @@ class Slither(SlitherCore): # pylint: disable=too-many-instance-attributes instance = printer_class(self, logger_printer) self._printers.append(instance) - def deregister_printer(self, printer_class: Type[AbstractPrinter]) -> None: + def unregister_printer(self, printer_class: Type[AbstractPrinter]) -> None: """ :param printer_class: Class inheriting from `AbstractPrinter`. """ From 5498e1b609b579d1210c626fa352025e9f41ca7e Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Thu, 9 Mar 2023 23:51:56 -0600 Subject: [PATCH 33/38] fix stdout capture Since `output_to_sarif` is a function and a "truthy" value, the capturing was enabled despite `outputting_sarif` being false. I've fixed the condition --- slither/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slither/__main__.py b/slither/__main__.py index 528a93e8f..48145a25d 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -760,7 +760,7 @@ def main_impl( # If we are outputting JSON, capture all standard output. If we are outputting to stdout, we block typical stdout # output. - if outputting_json or output_to_sarif: + if outputting_json or outputting_sarif: StandardOutputCapture.enable(outputting_json_stdout or outputting_sarif_stdout) printer_classes = choose_printers(args, all_printer_classes) From 8c58abe6ad3a7259a84adb448b0411927f73cf22 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 10 Mar 2023 11:18:51 +0100 Subject: [PATCH 34/38] Fix linters --- slither/printers/guidance/echidna.py | 4 ++-- slither/utils/type.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/slither/printers/guidance/echidna.py b/slither/printers/guidance/echidna.py index 0ee8a0901..b160dd0e6 100644 --- a/slither/printers/guidance/echidna.py +++ b/slither/printers/guidance/echidna.py @@ -12,6 +12,7 @@ from slither.core.declarations.solidity_variables import ( ) from slither.core.expressions import NewContract from slither.core.slither_core import SlitherCore +from slither.core.solidity_types import TypeAlias from slither.core.variables.state_variable import StateVariable from slither.core.variables.variable import Variable from slither.printers.abstract_printer import AbstractPrinter @@ -28,11 +29,10 @@ from slither.slithir.operations import ( InternalCall, TypeConversion, ) -from slither.core.solidity_types import TypeAlias, Type from slither.slithir.operations.binary import Binary from slither.slithir.variables import Constant -from slither.visitors.expression.constants_folding import ConstantFolding from slither.utils.output import Output +from slither.visitors.expression.constants_folding import ConstantFolding def _get_name(f: Union[Function, Variable]) -> str: diff --git a/slither/utils/type.py b/slither/utils/type.py index a6688b13e..1674999aa 100644 --- a/slither/utils/type.py +++ b/slither/utils/type.py @@ -1,7 +1,13 @@ import math from typing import List, Union, Set -from slither.core.solidity_types import ArrayType, MappingType, ElementaryType, UserDefinedType, TypeAlias +from slither.core.solidity_types import ( + ArrayType, + MappingType, + ElementaryType, + UserDefinedType, + TypeAlias, +) from slither.core.solidity_types.type import Type from slither.core.variables.variable import Variable @@ -88,7 +94,7 @@ def convert_type_for_solidity_signature(t: Type, seen: Set[Type]) -> Union[Type, for x in underlying_type.elems_ordered ] return types - + if isinstance(t, TypeAlias): return t.type From ae5af4ee00c2922db61ef7151485dead3d2c0f7e Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 10 Mar 2023 11:31:13 +0100 Subject: [PATCH 35/38] Print warning --- slither/__main__.py | 4 +++- slither/utils/command_line.py | 9 ++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/slither/__main__.py b/slither/__main__.py index d4b61da59..385dc608f 100644 --- a/slither/__main__.py +++ b/slither/__main__.py @@ -868,7 +868,9 @@ def main_impl( # Output our results to markdown if we wish to compile a checklist. if args.checklist: - output_results_to_markdown(results_detectors, args.checklist_limit) + output_results_to_markdown( + results_detectors, args.checklist_limit, args.show_ignored_findings + ) # Don't print the number of result for printers if number_contracts == 0: diff --git a/slither/utils/command_line.py b/slither/utils/command_line.py index 174c2a4b6..911609bf6 100644 --- a/slither/utils/command_line.py +++ b/slither/utils/command_line.py @@ -169,7 +169,9 @@ def convert_result_to_markdown(txt: str) -> str: return "".join(ret) -def output_results_to_markdown(all_results: List[Dict], checklistlimit: str) -> None: +def output_results_to_markdown( + all_results: List[Dict], checklistlimit: str, show_ignored_findings: bool +) -> None: checks = defaultdict(list) info: Dict = defaultdict(dict) for results_ in all_results: @@ -179,6 +181,11 @@ def output_results_to_markdown(all_results: List[Dict], checklistlimit: str) -> "confidence": results_["confidence"], } + if not show_ignored_findings: + print( + "**THIS CHECKLIST IS NOT COMPLETE**. Use `--show-ignored-findings` to show all the results." + ) + print("Summary") for check_ in checks: print( From 2f155e764e473e35a4854a3257ac3e674427eec6 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 10 Mar 2023 12:03:04 +0100 Subject: [PATCH 36/38] Improve tests from 1625 --- tests/conftest.py | 59 ------------------------------------------ tests/test_features.py | 12 ++++++--- 2 files changed, 9 insertions(+), 62 deletions(-) delete mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py deleted file mode 100644 index 85567af98..000000000 --- a/tests/conftest.py +++ /dev/null @@ -1,59 +0,0 @@ -import os -from contextlib import contextmanager -from pathlib import Path -from typing import Optional - -import pytest -from crytic_compile import CryticCompile -from crytic_compile.platform.solc_standard_json import SolcStandardJson -from solc_select import solc_select - -from slither import Slither - - -@contextmanager -def _select_solc_version(version: Optional[str]): - """Selects solc version to use for running tests. - If no version is provided, latest is used.""" - if not version: - # This sorts the versions numerically - vers = sorted( - map( - lambda x: (int(x[0]), int(x[1]), int(x[2])), - map(lambda x: x.split(".", 3), solc_select.installed_versions()), - ) - ) - ver = list(vers)[-1] - version = ".".join(map(str, ver)) - env = dict(os.environ) - env_restore = dict(env) - env["SOLC_VERSION"] = version - os.environ.clear() - os.environ.update(env) - - yield version - - os.environ.clear() - os.environ.update(env_restore) - - -@pytest.fixture(name="select_solc_version") -def fixture_select_solc_version(): - return _select_solc_version - - -@pytest.fixture -def slither_from_dir(select_solc_version): - @contextmanager - def _slither_from_dir(directory: str, solc_version: Optional[str] = None): - """Yields a Slither instance using solidity files in directory and solc_version. - Temporarily changes the solc-version temporary to solc_version. - """ - standard_json = SolcStandardJson() - for source_file in Path(directory).rglob("*.sol"): - standard_json.add_source_file(Path(source_file).as_posix()) - with select_solc_version(solc_version): - compilation = CryticCompile(standard_json) - yield Slither(compilation) - - return _slither_from_dir diff --git a/tests/test_features.py b/tests/test_features.py index b2a3960af..2716d8457 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -1,4 +1,5 @@ import inspect +from pathlib import Path from crytic_compile import CryticCompile from crytic_compile.platform.solc_standard_json import SolcStandardJson @@ -162,6 +163,11 @@ def test_arithmetic_usage() -> None: } == {"2b4bc73cf59d486dd9043e840b5028b679354dd9", "e4ecd4d0fda7e762d29aceb8425f2c5d4d0bf962"} -def test_using_for_global_collision(slither_from_dir) -> None: - with slither_from_dir("./tests/using-for-global-collision") as sl: - _run_all_detectors(sl) +def test_using_for_global_collision() -> None: + solc_select.switch_global_version("0.8.18", always_install=True) + standard_json = SolcStandardJson() + for source_file in Path("./tests/using-for-global-collision").rglob("*.sol"): + standard_json.add_source_file(Path(source_file).as_posix()) + compilation = CryticCompile(standard_json) + sl = Slither(compilation) + _run_all_detectors(sl) From 20ca59946d21718f4f7625936cb042b29e0f78f8 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Fri, 10 Mar 2023 08:06:35 -0600 Subject: [PATCH 37/38] lint --- slither/detectors/variables/predeclaration_usage_local.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/detectors/variables/predeclaration_usage_local.py b/slither/detectors/variables/predeclaration_usage_local.py index 2a24eed84..177035ef4 100644 --- a/slither/detectors/variables/predeclaration_usage_local.py +++ b/slither/detectors/variables/predeclaration_usage_local.py @@ -57,9 +57,9 @@ Additionally, the for-loop uses the variable `max`, which is declared in a previ # endregion wiki_exploit_scenario WIKI_RECOMMENDATION = "Move all variable declarations prior to any usage of the variable, and ensure that reaching a variable declaration does not depend on some conditional if it is used unconditionally." - + VULNERABLE_SOLC_VERSIONS = ALL_SOLC_VERSIONS_04 - + def detect_predeclared_local_usage( self, node: Node, From 7aba140c3d1d1014c9d91526aba515844a5e0065 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 10 Mar 2023 15:38:07 +0100 Subject: [PATCH 38/38] Test with legacy AST too --- tests/test_features.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_features.py b/tests/test_features.py index b42b60616..7c2db8a3e 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -95,6 +95,15 @@ def test_contract_comments() -> None: assert contract.comments == comments + # Test with legacy AST + comments = "@title Test Contract\n@dev Test comment" + solc_select.switch_global_version("0.5.16", always_install=True) + slither = Slither("./tests/custom_comments/contract_comment.sol", solc_force_legacy_json=True) + compilation_unit = slither.compilation_units[0] + contract = compilation_unit.get_contract_from_name("A")[0] + + assert contract.comments == comments + def test_using_for_top_level_same_name() -> None: solc_select.switch_global_version("0.8.15", always_install=True)