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

Reentrancy False Positive: Reported on internal function called by a nonReentrant external function #863

Closed
sudeepb02 opened this issue Jun 7, 2021 · 2 comments

Comments

@sudeepb02
Copy link

A Reentrancy vulnerability is reported by Slither in case an external nonReentrant function calls an internal function in the same contract.

For example:

function doSomething() internal {
    externalCallToAnotherContract();
}

function testExternal() nonReentrant external {
    doSomething();
}

Here as the doSomething is an internal function and only called inside a nonReentrant function, this should not be an issue.

@montyly
Copy link
Member

montyly commented Jun 9, 2021

Hi @sudeepb02. Thank you for reporting this.

It's actually more tricky to consider this as a FP, and it depends on the other functions. For example if you have doSomethingElse function that does not have the nonReentrant modifier, an attacker might be able to exploit the reentrancy by re-entering into doSomethingElse. For example:

function withdraw(uint amount) nonReentrant public{
     require(amount <= balances[msg.sender]);
     Receiver(msg.sender).send_funds({value: amount});
     balances[msg.sender] -= amount;
}

function withdraw_all() public{
     uint amount = balances[msg.sender];
     balances[msg.sender] = 0;
     Receiver(msg.sender).send_funds({value: amount});
}

However, we are working on a custom flag to tell Slither that all functions marked nonReentrant should be considered as safe: #854

@sudeepb02
Copy link
Author

This is interesting, thanks for the info. But in my opinion allowing to set a custom flag would just mask the issue and not actually solve it.

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

No branches or pull requests

3 participants