Potential false positive for reentrancy when calling the same contract

[ptare]

When a method calls back into the same contract by using this, Slither picks that up as a potential reentrancy bug. See this example [1] and the error produced [2].

Calling into the same contract isn't giving control to some external, untrusted contract, so this pattern could be considered safe. On the other hand, I do understand what the detector is flagging, so please feel free to close this issue if the behavior is intended. (Is there a way to suppress or workaround the warning?)

Thanks!

/*
  A self-contained example of false positive on calling back into the contract.
*/
pragma solidity ^0.5.0;

contract SelfCaller {
    uint public stateVariable;
    uint public stateVariable2;

    function a() external {
        this.b();
        stateVariable = 0;
    }

    function b() external {
        stateVariable2 = 5;
    }
}

Reentrancy in SelfCaller.a (MinimalThisExample.sol#10-13):
	External calls:
	- this.b() (MinimalThisExample.sol#11)
	External calls sending eth:
	State variables written after the call(s):
	- stateVariable (MinimalThisExample.sol#12)

[montyly]

Hi @ptare

That's another interesting corner case, thank for reporting it!

So I think that the right way to prevent this FP, is for slither to consider external calls to this as internal function calls. The reason is that this.b() could potentially do a call to an external contract and so be a true positive (but slither can determine if it does)

[montyly]

Fixed in 02661eb (dev branch)

Slither will now make the distinction between a call to this which is reentrancy-safe from one which is not:

contract SelfCaller {
    uint public stateVariable;
    uint public stateVariable2;

    address addr;

    function good() external {
        this.no_external_call();
        stateVariable = 0;
    }

    function no_external_call() external {
        stateVariable2 = 5;
    }


   function bad() external{
        this.external_call();
        stateVariable = 0;
    }

    function external_call() external{
        call();
    }   

    function call() internal{
        addr.call();
    }
}

bad is detected, but not good.

Thank you @ptare for reporting this edge-case!