Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support // slither-disable-start <detectors> and // slither-disable-end <detectors> #1453

Closed
mds1 opened this issue Nov 5, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@mds1
Copy link
Contributor

mds1 commented Nov 5, 2022

Describe the desired feature

Right now you can only disable detectors for the next line, but it would very helpful to be able to disable them for larger blocks of code. A sample use case is writing solidity scripts with foundry: reentrancy is not a concern at all so I can disable reentrancy for the file, but I still want to run uninitialized variable checks to avoid passing the zero address as function inputs.

I feel like this shouldn't be too hard to implement since // slither-disable-next-line is already implemented, so if you can provide some guidance on how to add this I'm happy to take a shot to get this merged sooner 🙂

Sample usage to disable some reentrancy checks for the full file (I think this only disables one category of reentrancy checks, not sure if there's a shorthand for disabling all of them)

// slither-disable-start reentrancy-benign

pragma solidity ^0.8.16;

import "forge-std/Script.sol";
import {Counter} from "src/Counter.sol";

contract Deploy is Script {
  Counter counter;

  function run() public {
    vm.startBroadcast();
    counter = new Counter();
  }
}
@mds1 mds1 added the enhancement New feature or request label Nov 5, 2022
@0xalpharush
Copy link
Contributor

0xalpharush commented Nov 7, 2022

Right now, the results get filtered out here:

if self.has_ignore_comment(r):

It currently looks at the previous line of the result:
for file, lines in mapping_elements_with_lines:
ignore_line_index = min(lines) - 1
ignore_line_text = self.crytic_compile.get_code_from_line(file, ignore_line_index)
if ignore_line_text:
match = re.findall(
r"^\s*//\s*slither-disable-next-line\s*([a-zA-Z0-9_,-]*)",
ignore_line_text.decode("utf8"),
)
if match:
ignored = match[0].split(",")
if ignored and ("all" in ignored or any(r["check"] == c for c in ignored)):
return True

I think it'd make sense to create and memoize a dict for each file that records what checks are disabled , and filter based off that.

if result["check"] in ignore_file_dict[file]:
  check if elems in results["elements"] overlap with range of start-end or have ignore comment on previous line 

This result is generated by each detector calling generate_result and the content depends on what was passed, e.g. a function or variable.

def generate_result(

related #745

Hopefully, this background info is helpful. I'm perfectly happy to answer follow-up questions if you decide to implement this.

@mds1
Copy link
Contributor Author

mds1 commented Nov 8, 2022

Thanks! That was super helpful, just opened #1461

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants