understanding the sGuard's output when muptiple contracts exist in a source file

[sunbeomso]

Hi,

Could you give some help on understanding sGuard's output?

Suppose a below test contract file is given as input:

pragma solidity^0.4.26;
  
contract Fund {
  mapping(address => uint) balances;
  uint counter = 0;
  uint dontFixMe = 0;

  function main(uint x) public {
    if (counter < 100) {
      msg.sender.send(x + 1);
      counter += 1;
      dontFixMe ++;
    }
  }
}

contract Fund2 {
  mapping(address => uint) balances;
  uint counter = 0;
  uint dontFixMe = 0;

  function main(uint x) public {
    if (counter < 100) {
      msg.sender.send(x + 1);
      counter += 1;
      dontFixMe ++;
    }
  }
}


contract Fund3 {
  mapping(address => uint) balances;
  uint counter = 0;
  uint dontFixMe = 0;

  function main(uint x) public {
    if (counter < 100) {
      msg.sender.send(x + 1);
      counter += 1;
      dontFixMe ++;
    }
  }
}

When I designate Fund2 contract as main, the sGuard's output is the following:

contract sGuard{
  function add_uint256(uint256 a, uint256 b) internal pure returns (uint256) {
    uint256 c = a + b;
    assert(c >= a);
    return c;
  }

  bool internal locked_;
  constructor() internal {
    locked_ = false;
  }
  modifier nonReentrant_() {
    require(!locked_);
    locked_ = true;
    _;
    locked_ = false;
  }
}
pragma solidity^0.4.26;

contract Fund  is sGuard {
  mapping(address => uint) balances;
  uint counter = 0;
  uint dontFixMe = 0;

   function main(uint x) nonReentrant_  public {
    if (counter < 100) {
      msg.sender.send(x + 1);
      counter += 1;
      dontFixMe ++;
    }
  }
}

contract Fund2  is sGuard {
  mapping(address => uint) balances;
  uint counter = 0;
  uint dontFixMe = 0;

  function main(uint x) public {
    if (counter < 100) {
      msg.sender.send(add_uint256(x, 1));
      counter = add_uint256(counter, 1);
      dontFixMe ++;
    }
  }
}


contract Fund3  is sGuard {
  mapping(address => uint) balances;
  uint counter = 0;
  uint dontFixMe = 0;

  function main(uint x) public {
    if (counter < 100) {
      msg.sender.send(x + 1);
      counter += 1;
      dontFixMe ++;
    }
  }
}

My questions are the following: [Q1] I wonder why sGuard added "nonReentrant_" modifier to the function in Fund (not the function in Fund2). As mentioned above, the specified main contract is Fund2.

[Q2] Independent from Q1, I wonder whether sGuard considers "send" instructions as sources of reentrancy vulnerabilities. To my knowledge, we cannot reenter to some functions using "send" due to gas fee restrictions on "send". However, sGuard seems to consider that the reentrancy is possible using "send".

[duytai]

hi @SunBeomSo [Q1] This could be many functions have the same signature, can you rename the function main in contract Fund3, for example: function main3(uint x) .... [Q2] I agree that it is not reentrancy bug. However, send is translated to opcode CALL. It is an external call then I generate the fix (the result is over-approximated)

[sunbeomso]

@duytai

Thanks for the answer. Regarding the answer for [Q1], it seems all functions that have the same signatures in different contracts should be renamed when using sGuard. I think this issue can be solved if sGuard performs modifications on functions within a main contract. Do you have any plan to address this issue in sGuard tool?

[duytai]

@SunBeomSo I will inform you when the problem is solved