crytic / slither

Static Analyzer for Solidity and Vyper
https://blog.trailofbits.com/2018/10/19/slither-a-solidity-static-analysis-framework/
GNU Affero General Public License v3.0
5.32k stars 964 forks source link

[False Negative]: reentrancy-no-eth for interprocedural case #2544

Open 0xalpharush opened 2 months ago

0xalpharush commented 2 months ago

What bug did Slither miss and which detector did you anticipate would catch it?

The following should be detected by reentrancy-no-eth but it is incorrectly considered benign. It is more important to highlight that there is a read-call-write than a write after a call

Frequency

Very Frequently

Code example to reproduce the issue:

contract T1 {
 uint x;
 function re() external {
   require(x != 9);
   re2(); 
 }
  function re2() internal {
   address(1).call("");
   x = 9;
 }

}

Version:

0.10.4

Relevant log output:

INFO:Detectors:
T1.re2() (t.sol#7-10) ignores return value by address(1).call() (t.sol#8)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-low-level-calls
INFO:Detectors:
Reentrancy in T1.re2() (t.sol#7-10):
    External calls:
    - address(1).call() (t.sol#8)
    State variables written after the call(s):
    - x = 9 (t.sol#9)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2
INFO:Detectors:
Low level call in T1.re2() (t.sol#7-10):
    - address(1).call() (t.sol#8)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#low-level-calls
0xalpharush commented 2 months ago

Related https://github.com/crytic/slither/issues/1566