Merge pull request #1497 from crytic/dev-protected-var

Improve protected variable detector
pull/1510/head
Feist Josselin 2 years ago committed by GitHub
commit 023f49d9e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      slither/detectors/functions/protected_variable.py
  2. 19
      tests/detectors/protected-vars/0.8.2/comment.sol
  3. 183
      tests/detectors/protected-vars/0.8.2/comment.sol.0.8.2.ProtectedVariables.json

@ -48,7 +48,7 @@ contract Buggy{
def _analyze_function(self, function: Function, contract: Contract) -> List[Output]: def _analyze_function(self, function: Function, contract: Contract) -> List[Output]:
results = [] 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: if state_variable_written.write_protection:
for function_sig in state_variable_written.write_protection: for function_sig in state_variable_written.write_protection:
function_protection = contract.get_function_from_signature(function_sig) function_protection = contract.get_function_from_signature(function_sig)

@ -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;
}
}

@ -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": [ "elements": [
{ {

Loading…
Cancel
Save