Injected reentrancy bugs detectable as different bug classes

[f0rki]

Various of the reentrancy bugs injected in this dataset are also detectable as a different bug class. For example, several of the reentrancy bugs are detected by MAIAN, an analysis tool that does not attempt to identify reentrancy. This can be misleading when using this dataset for tool evaluation. Tools might detect certain bugs due to different reasons, complicating a comparison of the tools.

For example, the following pattern is injected into the reentrancy dataset:

address payable lastPlayer_re_ent9;
uint jackpot_re_ent9;
function buyTicket_re_ent9() public {
  (bool success,) = lastPlayer_re_ent9.call.value(jackpot_re_ent9)("");
  if (!success)
    revert();
  lastPlayer_re_ent9 = msg.sender;
  jackpot_re_ent9    = address(this).balance;
}

However, there is absolutely no need to perform reentrancy here. In the first invocation lastPlayer_re_ent9 is 0 initialized. So the external call will simply succeed without causing any reentrancy. However, also no ether will be transferred. Then lastPlayer_re_ent9 is set to the attacker and jackpot_re_ent9 is set to all the ether available. The next call to this function will transfer the total ether balance of the victim contract. But where is the reentrancy here? In theory the code can cause reentrancy, but it would not make sense as part of an attack.

In the inspection.py script it seems that for Manticore, 'Reachable ether leak to sender' is considered as a bug code for reentrancy (see here). Why was this done? In the example above Manticore, and other analysis tools like MAIAN or teEther, will identify a leaking Ether issue, not a reentrancy bug. In my opinion this is also the correct bug class for the code above.

I believe the following bug pattern has a similar issue, although here the attacker can actually utilize reentrancy to steal more than 1 ether.

bool not_called_re_ent27 = true;
function bug_re_ent27() public{
    require(not_called_re_ent27);
    if( ! (msg.sender.send(1 ether) ) ){
        revert();
    }
    not_called_re_ent27 = false;
}

In general it seems a bit problematic to inject bug patterns that can be identified as two different bug classes, as it complicates comparisons between tools. This might invalidate some of the results presented in the paper. I suggest to double check the results and maybe publish an addendum to the paper, if necessary.