Minor improvements + testcase

pull/1483/head
Josselin Feist 2 years ago
parent a16202af02
commit 3880ad63a1
  1. 23
      slither/core/slither_core.py
  2. 22
      tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol
  3. 231
      tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol.0.8.10.ReentrancyEth.json
  4. 3
      tests/test_detectors.py

@ -76,8 +76,6 @@ class SlitherCore(Context):
self._ignore_ranges: defaultdict[str, defaultdict[str, List[(int, int)]]] = defaultdict(
lambda: defaultdict(lambda: [])
)
# Track which files for _ignore_ranges have been processed (a processed file may have no entries in _ignore_ranges).
self._processed_ignores: Set[str] = set()
self._compilation_units: List[SlitherCompilationUnit] = []
@ -159,7 +157,7 @@ class SlitherCore(Context):
def filename(self, filename: str):
self._filename = filename
def add_source_code(self, path):
def add_source_code(self, path: str) -> None:
"""
:param path:
:return:
@ -170,6 +168,8 @@ class SlitherCore(Context):
with open(path, encoding="utf8", newline="") as f:
self.source_code[path] = f.read()
self.parse_ignore_comments(path)
@property
def markdown_root(self) -> str:
return self._markdown_root
@ -292,12 +292,10 @@ class SlitherCore(Context):
###################################################################################
###################################################################################
def parse_ignore_comments(self, file: str):
def parse_ignore_comments(self, file: str) -> None:
# The first time we check a file, find all start/end ignore comments and memoize them.
line_number = 1
while True:
if file in self._processed_ignores:
break
line_text = self.crytic_compile.get_code_from_line(file, line_number)
if line_text is None:
@ -317,9 +315,10 @@ class SlitherCore(Context):
# First item in the array, or the prior item is fully populated.
self._ignore_ranges[file][check].append((line_number, float("inf")))
else:
raise Exception(
"consecutive slither-disable-starts without slither-disable-end"
logger.error(
f"Consecutive slither-disable-starts without slither-disable-end in {file}#{line_number}"
)
return
if end_match:
ignored = end_match[0].split(",")
@ -327,13 +326,14 @@ class SlitherCore(Context):
for check in ignored:
vals = self._ignore_ranges[file][check]
if len(vals) == 0 or vals[-1][1] != float("inf"):
raise Exception("slither-disable-end without slither-disable-start")
logger.error(
f"slither-disable-end without slither-disable-start in {file}#{line_number}"
)
return
self._ignore_ranges[file][check][-1] = (vals[-1][0], line_number)
line_number += 1
self._processed_ignores.add(file)
def has_ignore_comment(self, r: Dict) -> bool:
"""
Check if the result has an ignore comment in the file or on the preceding line, in which
@ -354,7 +354,6 @@ class SlitherCore(Context):
)
for file, lines in mapping_elements_with_lines:
self.parse_ignore_comments(file)
# Check if result is within an ignored range.
ignore_ranges = self._ignore_ranges[file][r["check"]] + self._ignore_ranges[file]["all"]

@ -0,0 +1,22 @@
interface Receiver{
function send_funds() payable external;
}
contract TestWithBug{
mapping(address => uint) balances;
function withdraw(uint amount) public{
require(amount <= balances[msg.sender]);
Receiver(msg.sender).send_funds{value: amount}();
balances[msg.sender] -= amount;
}
// slither-disable-start all
function withdrawFiltered(uint amount) public{
require(amount <= balances[msg.sender]);
Receiver(msg.sender).send_funds{value: amount}();
balances[msg.sender] -= amount;
}
// slither-disable-end all
}

@ -0,0 +1,231 @@
[
[
{
"elements": [
{
"type": "function",
"name": "withdraw",
"source_mapping": {
"start": 133,
"length": 194,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
8,
9,
10,
11,
12
],
"starting_column": 5,
"ending_column": 6
},
"type_specific_fields": {
"parent": {
"type": "contract",
"name": "TestWithBug",
"source_mapping": {
"start": 67,
"length": 534,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21
],
"starting_column": 1,
"ending_column": 2
}
},
"signature": "withdraw(uint256)"
}
},
{
"type": "node",
"name": "Receiver(msg.sender).send_funds{value: amount}()",
"source_mapping": {
"start": 231,
"length": 48,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
10
],
"starting_column": 10,
"ending_column": 58
},
"type_specific_fields": {
"parent": {
"type": "function",
"name": "withdraw",
"source_mapping": {
"start": 133,
"length": 194,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
8,
9,
10,
11,
12
],
"starting_column": 5,
"ending_column": 6
},
"type_specific_fields": {
"parent": {
"type": "contract",
"name": "TestWithBug",
"source_mapping": {
"start": 67,
"length": 534,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21
],
"starting_column": 1,
"ending_column": 2
}
},
"signature": "withdraw(uint256)"
}
}
},
"additional_fields": {
"underlying_type": "external_calls"
}
},
{
"type": "node",
"name": "balances[msg.sender] -= amount",
"source_mapping": {
"start": 290,
"length": 30,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
11
],
"starting_column": 10,
"ending_column": 40
},
"type_specific_fields": {
"parent": {
"type": "function",
"name": "withdraw",
"source_mapping": {
"start": 133,
"length": 194,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
8,
9,
10,
11,
12
],
"starting_column": 5,
"ending_column": 6
},
"type_specific_fields": {
"parent": {
"type": "contract",
"name": "TestWithBug",
"source_mapping": {
"start": 67,
"length": 534,
"filename_relative": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"filename_absolute": "/GENERIC_PATH",
"filename_short": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol",
"is_dependency": false,
"lines": [
5,
6,
7,
8,
9,
10,
11,
12,
13,
14,
15,
16,
17,
18,
19,
20,
21
],
"starting_column": 1,
"ending_column": 2
}
},
"signature": "withdraw(uint256)"
}
}
},
"additional_fields": {
"underlying_type": "variables_written",
"variable_name": "balances"
}
}
],
"description": "Reentrancy in TestWithBug.withdraw(uint256) (tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#8-12):\n\tExternal calls:\n\t- Receiver(msg.sender).send_funds{value: amount}() (tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#10)\n\tState variables written after the call(s):\n\t- balances[msg.sender] -= amount (tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#11)\n\tTestWithBug.balances (tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#6) can be used in cross function reentrancies:\n\t- TestWithBug.withdraw(uint256) (tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#8-12)\n\t- TestWithBug.withdrawFiltered(uint256) (tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#15-19)\n",
"markdown": "Reentrancy in [TestWithBug.withdraw(uint256)](tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L8-L12):\n\tExternal calls:\n\t- [Receiver(msg.sender).send_funds{value: amount}()](tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L10)\n\tState variables written after the call(s):\n\t- [balances[msg.sender] -= amount](tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L11)\n\t[TestWithBug.balances](tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L6) can be used in cross function reentrancies:\n\t- [TestWithBug.withdraw(uint256)](tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L8-L12)\n\t- [TestWithBug.withdrawFiltered(uint256)](tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L15-L19)\n",
"first_markdown_element": "tests/detectors/reentrancy-eth/0.8.10/reentrancy_filtered_comments.sol#L8-L12",
"id": "176d2b5b09c260c72fd638ff8b5db4709df3ff3eb253daa1cfde254c8299fb94",
"check": "reentrancy-eth",
"impact": "High",
"confidence": "Medium"
}
]
]

@ -362,7 +362,10 @@ ALL_TEST_OBJECTS = [
"DAO.sol",
"0.4.25",
),
# Test the nonReentrant filtering
Test(all_detectors.ReentrancyEth, "reentrancy_with_non_reentrant.sol", "0.8.10"),
# Test parse_ignore_comments
Test(all_detectors.ReentrancyEth, "reentrancy_filtered_comments.sol", "0.8.10"),
Test(
all_detectors.UninitializedStorageVars,
"uninitialized_storage_pointer.sol",

Loading…
Cancel
Save