0

I just saw something like this:

function sub(
    uint a,
    uint b
    )
    internal
    pure
    returns (uint)
{
    require(b <= a, "SUB_UNDERFLOW");
    return a - b;
}

function withdraw(uint amount) public {
    msg.sender.call.value(amount).gas(gasleft())("");
    balance[msg.sender]=balance[msg.sender].sub(amount);
}

Is there a possible reentrancy attack here, or does balance[msg.sender] is reloaded after the call making the subtraction based on the value after the call?
Can compiler optimizations affect the behavior through caching on the stack in order to have a single SLOAD operation in terms of gas cost?

user2284570
  • 1,008
  • 1
  • 12
  • 30

1 Answers1

1

Yes, there's the possibility of a re-entrancy attack

function withdraw(uint amount) public {
    msg.sender.call.value(amount).gas(gasleft())("");
    balance[msg.sender]=balance[msg.sender].sub(amount);
}

The call to msg.sender will execute the fallback function and it may call another contract.

Since the balance is not adjusted yet temporarily the contract still has positive balance and ether transfered.

You cannot exploit re-entrancy with that only but still it is quite dangerous.

It is recommended in several places to follow the pattern checks-effects-interactions to avoid this kind of vulnerabilities.

Ismael
  • 30,570
  • 21
  • 53
  • 96
  • You cannot exploit re-entrancy with that only but still it is quite dangerous. Because balance[msg.sender] value is refetched from storage after the contract call in reentrancy? – user2284570 Apr 21 '20 at 11:46
  • @user2284570 The value is never fetched before the transfer, it is only fetched after it. The problem of reentrancy is that the user momentarily has gained control and the balance doesn't reflect the real value. If some other function of your contract relies on that invariant it might be exploited. – Ismael Apr 21 '20 at 13:47
  • In that case, I was talking only about the withdraw function itself. Am I correct to assume there’s no vulnerability in that case? If yes, why? – user2284570 Apr 21 '20 at 14:44
  • In "theory" the withdraw function alone cannot be attacked. But together with another bug it might be exploitable. – Ismael Apr 22 '20 at 03:52