slither icon indicating copy to clipboard operation
slither copied to clipboard

Improve reentrancy detectors

Open montyly opened this issue 3 years ago • 1 comments

  • Filter result based on nonReentrant modifier
    • reentrancy-eth/reentrancy-no-eth: do not warn if the function is non reentrant, and there is no other reentrant function that writes to the affected variable
    • reentrancy-event: do not warm in the function is non reentrant.
    • Fix https://github.com/crytic/slither/issues/863, https://github.com/crytic/slither/issues/683. Replace https://github.com/crytic/slither/issues/765
  • Fix incorrect detection on loop (fix https://github.com/crytic/slither/issues/1019)
  • Improve python types
  • Add relevant APIs for the reentrancy filtering:
    • function.all_reachable_from_functions list of all the functions in the call graph that leads to function
    • function.is_reentrant: check if the function is reentrant. Here we check if the nonReentrant modifier is directly applied, or for internal functions if one of the entry points (i.e. one of the public/external function that can reach the targeted function) is lacking nonReentrant

I might also improve the reentrancy-event filtering to detect cross-functions reentrancy on event - through this is usually only informational and have a limited impact.

Additionally right now nonReentrant is only checked based on the modifier name. We could compare the source code hash of the known version to increase the confidence.

This require additional internal testing before being merged.

montyly avatar Aug 17 '22 12:08 montyly

TODO: replace is no other reentrant function that writes to the affected variable to include read, and state variable that are public.

This will help reviewing cross protocol reentrancies

montyly avatar Aug 17 '22 17:08 montyly