OpenZeppelin ERC20: IllegalArgumentException

[ajhodges]

I'm running into an error when scanning a contract that references the OpenZeppelin ERC20 contract:

Exception in thread "main" java.lang.IllegalArgumentException: fromIndex(0) > toIndex(-1)
    at java.util.ArrayList.subListRangeCheck(ArrayList.java:1014)
    at java.util.ArrayList.subList(ArrayList.java:1004)
    at ch.securify.CompilationHelpers.bytecodeOffsetToSourceOffset(CompilationHelpers.java:59)
    at ch.securify.CompilationHelpers.getMatchedLines(CompilationHelpers.java:111)
    at ch.securify.CompilationHelpers.getMappingsFromStatusFile(CompilationHelpers.java:135)
    at ch.securify.Main.processSolidityFile(Main.java:110)
    at ch.securify.Main.main(Main.java:186)

Steps to reproduce:

1. Download the OpenZeppelin ERC20.sol
2. Run Securify as documented in the readme (docker run -v $(pwd)/folder_with_solidity_files:/contracts securify)

I have a feeling this could be due to the fact that ERC20.sol is an abstract contract. I am referencing the ERC20 abstract contract in another contract that accepts ERC20 token transfers.

[hiqua]

If you run solc ERC20.sol it will fail because of the imports, right? Given that Securify uses solc under the hood, you cannot expect it to work like this.

[ajhodges]

Sorry, I'm actually using an older version of ERC20.sol that doesn't have imports:

pragma solidity ^0.4.24;

/**
 * @title ERC20 interface
 * @dev see https://github.com/ethereum/EIPs/issues/20
 */
contract ERC20 {
  function totalSupply() public view returns (uint256);

  function balanceOf(address _who) public view returns (uint256);

  function allowance(address _owner, address _spender)
    public view returns (uint256);

  function transfer(address _to, uint256 _value) public returns (bool);

  function approve(address _spender, uint256 _value)
    public returns (bool);

  function transferFrom(address _from, address _to, uint256 _value)
    public returns (bool);

  event Transfer(
    address indexed from,
    address indexed to,
    uint256 value
  );

  event Approval(
    address indexed owner,
    address indexed spender,
    uint256 value
  );
}

It looks like they've been changing things and I didn't notice when I linked to it. Either way, the soljitsu project I am using flattens all of my truffle dependencies into a single directory. All of the code compiles correctly with solc and passes lint checks with solhint.

[hiqua]

Indeed it's related to the fact that this contract is abstract, which Securify does not support given that it relies on solc (which will produce an empty output in this case). I'll add a note for this in the README. I would say that it's unlikely that abstract contracts will be supported anytime soon (in practice, people have at least a concrete contract inheriting from the abstract one to be able to test it).

[ajhodges]

I just want to add that in my case, I am not trying to run securify on just ERC20, I am trying to run securify on a contract that references an ERC20 token and uses the abstract contract to perform transferFrom() on an existing token. Like this (dumbed-down pseudocode):

import {ERC20} from "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";

function erc20Transfer(address from, uint256 amount, address token)
        public
    {
        if (!ERC20(token).transferFrom(from, address(this), amount)) {
            revert();
        }
    }

[hiqua]

Unless I'm mistaken this contract is abstract as well given that it doesn't implement all the virtual functions from the parent contract, so the same remark would apply.

[ajhodges]

In this case, the address token would be the address of a token that implements the ERC20 standard. I am using the ERC20 interface to represent an abstraction of any token that implements transferFrom. In this example, I don't really care which token I'm calling transferFrom on, just that the method gets called. That's my use case for referencing an abstract contract in the implementation of another contract.

[hiqua]

Ok I see, so the two contracts to reproduce this are:

import {ERC20} from "./ERC20.sol";

contract C {
function erc20Transfer(address from, uint256 amount, address token)
        public
    {
        if (!ERC20(token).transferFrom(from, address(this), amount)) {
            revert();
        }
    }

}

and

import {ERC20} from "./ERC20.sol";

contract C {
function erc20Transfer(address from, uint256 amount, address token)
        public
    {
        if (!ERC20(token).transferFrom(from, address(this), amount)) {
            revert();
        }
    }

}

The bug is not present on another branch I'm working on now, so just wait a bit and the bug should disappear. It's probably related to the source mappings.