diff --git a/slither/detectors/functions/protected_variable.py b/slither/detectors/functions/protected_variable.py index 0c1341d3ca..cbd640e18e 100644 --- a/slither/detectors/functions/protected_variable.py +++ b/slither/detectors/functions/protected_variable.py @@ -48,7 +48,7 @@ class ProtectedVariables(AbstractDetector): def _analyze_function(self, function: Function, contract: Contract) -> List[Output]: results = [] - for state_variable_written in function.state_variables_written: + for state_variable_written in function.all_state_variables_written(): if state_variable_written.write_protection: for function_sig in state_variable_written.write_protection: function_protection = contract.get_function_from_signature(function_sig) diff --git a/tests/detectors/protected-vars/0.8.2/comment.sol b/tests/detectors/protected-vars/0.8.2/comment.sol index e06ff3902c..a49558b611 100644 --- a/tests/detectors/protected-vars/0.8.2/comment.sol +++ b/tests/detectors/protected-vars/0.8.2/comment.sol @@ -33,3 +33,22 @@ contract ReentrancyAndWrite{ } } +contract Internal { + /// @custom:security write-protection="onlyOwner()" + address owner; + + + + modifier onlyOwner(){ + // lets assume there is an access control + _; + } + + function buggy() public { + internal_write(); + } + + function internal_write() internal { + owner = msg.sender; + } +} \ No newline at end of file diff --git a/tests/detectors/protected-vars/0.8.2/comment.sol.0.8.2.ProtectedVariables.json b/tests/detectors/protected-vars/0.8.2/comment.sol.0.8.2.ProtectedVariables.json index cb1ead6217..2c1a11fa40 100644 --- a/tests/detectors/protected-vars/0.8.2/comment.sol.0.8.2.ProtectedVariables.json +++ b/tests/detectors/protected-vars/0.8.2/comment.sol.0.8.2.ProtectedVariables.json @@ -1,5 +1,188 @@ [ [ + { + "elements": [ + { + "type": "function", + "name": "buggy", + "source_mapping": { + "start": 938, + "length": 57, + "filename_relative": "tests/detectors/protected-vars/0.8.2/comment.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/protected-vars/0.8.2/comment.sol", + "is_dependency": false, + "lines": [ + 47, + 48, + 49 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Internal", + "source_mapping": { + "start": 742, + "length": 331, + "filename_relative": "tests/detectors/protected-vars/0.8.2/comment.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/protected-vars/0.8.2/comment.sol", + "is_dependency": false, + "lines": [ + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "buggy()" + } + }, + { + "type": "function", + "name": "onlyOwner", + "source_mapping": { + "start": 844, + "length": 88, + "filename_relative": "tests/detectors/protected-vars/0.8.2/comment.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/protected-vars/0.8.2/comment.sol", + "is_dependency": false, + "lines": [ + 42, + 43, + 44, + 45 + ], + "starting_column": 5, + "ending_column": 6 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Internal", + "source_mapping": { + "start": 742, + "length": 331, + "filename_relative": "tests/detectors/protected-vars/0.8.2/comment.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/protected-vars/0.8.2/comment.sol", + "is_dependency": false, + "lines": [ + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55 + ], + "starting_column": 1, + "ending_column": 0 + } + }, + "signature": "onlyOwner()" + } + }, + { + "type": "variable", + "name": "owner", + "source_mapping": { + "start": 822, + "length": 13, + "filename_relative": "tests/detectors/protected-vars/0.8.2/comment.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/protected-vars/0.8.2/comment.sol", + "is_dependency": false, + "lines": [ + 38 + ], + "starting_column": 5, + "ending_column": 18 + }, + "type_specific_fields": { + "parent": { + "type": "contract", + "name": "Internal", + "source_mapping": { + "start": 742, + "length": 331, + "filename_relative": "tests/detectors/protected-vars/0.8.2/comment.sol", + "filename_absolute": "/GENERIC_PATH", + "filename_short": "tests/detectors/protected-vars/0.8.2/comment.sol", + "is_dependency": false, + "lines": [ + 36, + 37, + 38, + 39, + 40, + 41, + 42, + 43, + 44, + 45, + 46, + 47, + 48, + 49, + 50, + 51, + 52, + 53, + 54, + 55 + ], + "starting_column": 1, + "ending_column": 0 + } + } + } + } + ], + "description": "Internal.buggy() (tests/detectors/protected-vars/0.8.2/comment.sol#47-49) should have Internal.onlyOwner() (tests/detectors/protected-vars/0.8.2/comment.sol#42-45) to protect Internal.owner (tests/detectors/protected-vars/0.8.2/comment.sol#38)\n", + "markdown": "[Internal.buggy()](tests/detectors/protected-vars/0.8.2/comment.sol#L47-L49) should have [Internal.onlyOwner()](tests/detectors/protected-vars/0.8.2/comment.sol#L42-L45) to protect [Internal.owner](tests/detectors/protected-vars/0.8.2/comment.sol#L38)\n", + "first_markdown_element": "tests/detectors/protected-vars/0.8.2/comment.sol#L47-L49", + "id": "347d5dbdb03710066bc29d7772156fe5ff3d3371fa4eee4839ee221a1d0de0a4", + "check": "protected-vars", + "impact": "High", + "confidence": "High" + }, { "elements": [ {