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

Injected reentrancy bugs are not exploitable #3

Open
f0rki opened this issue Apr 28, 2022 · 0 comments
Open

Injected reentrancy bugs are not exploitable #3

f0rki opened this issue Apr 28, 2022 · 0 comments

Comments

@f0rki
Copy link

f0rki commented Apr 28, 2022

Most of the injected bug patterns for reentrancy are not exploitable, or are even effectively dead code. This seems like a major drawback of this dataset when trying to evaluate reentrancy detection tools: the more precise a tool becomes, the worse it will perform on this dataset. In my opinion this is quite misleading.

The following pattern is injected into many of the contracts in the reentrancy category:

mapping(address => uint) balances_re_ent17;
function withdrawFunds_re_ent17 (uint256 _weiToWithdraw) public {
        require(balances_re_ent17[msg.sender] >= _weiToWithdraw);
        // limit the withdrawal
        (bool success,)=msg.sender.call.value(_weiToWithdraw)("");
        require(success);  //bug
        balances_re_ent17[msg.sender] -= _weiToWithdraw;
}

Now here balances_re_ent17 is 0 initialized as all datatypes in Solidity/Ethereum. There is no way to change the values in balances_re_ent17. As such, the only valid call that does pass the require statement in the beginning of withdrawFunds_re_ent17 is to pass _weiToWithdraw == 0 as a parameter. This will transfer 0 ether. So one can reenter as much as one likes by always transferring 0 ether and subtracting 0 from 0. Not very useful and definitely not exploitable.

The next reentrancy pattern is broken in two ways:

  • Effectively dead code
  • Uses transfer, where reentrancy is not possible.
// 0 initialized and no function to update the mapping
mapping(address => uint) redeemableEther_re_ent25;
function claimReward_re_ent25() public {        
      // can never be satisfied
      require(redeemableEther_re_ent25[msg.sender] > 0);
      // unreachable
      uint transferValue_re_ent25 = redeemableEther_re_ent25[msg.sender];
      // msg.sender.transfer does not allow for reentrancy due to gas limits
      msg.sender.transfer(transferValue_re_ent25);   //bug
      redeemableEther_re_ent25[msg.sender] = 0;
}

I suggest to put a big disclaimer somewhere that this dataset should not be used to evaluate reentrancy detection.

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

1 participant