Decompilation errors with some contracts

[hiqua]

Some contracts trigger decompilation errors, which prevent any analysis. The output is then empty. At least Securify should fail loudly to prevent the false sense of Securify.

In the example I have, modifying some statements a bit (e.g. by using a temporary variable to rewrite one line into two) is enough to work around the issue. Using solc with the --optimize flag also works.

[hiqua]

Related to #32.

[hiqua]

Low priority for me because @ritzdorf is currently working on this, so I'll let him finish his PR unless I'm done with the rest.

[MatthiasEgli-chainsecurity]

Lets make this one the official issue to track the proposed changes for the big PR #32. For remaining issues after #32 lets open new issues.

[MatthiasEgli-chainsecurity]

Also lets be more verbose in writing issues, like specifying which contract failed.

[hiqua]

Regarding your last comment, the main problem is when it's code we cannot make public.

[MatthiasEgli-chainsecurity]

There usually are minimal examples which aren't violating private code

[hiqua]

contract ERC20 { }

contract ExpiringMarket {
    function offer(uint) public returns (uint ) {}
}

contract MatchingMarket is ExpiringMarket {
    function offer(uint) public returns (uint) {
        function(uint) returns(uint256) fn = true ? _offeru : super.offer;
        return fn(0);
    }

    function o(uint pay_amt, uint) public {
        super.offer(pay_amt);
    }

    function _offeru(uint) internal returns (uint) {}
}

triggers

java.lang.NullPointerException
        at ch.securify.decompiler.DestackerFallback.findJumpCondition(DestackerFallback.java:403)
        at ch.securify.decompiler.DestackerFallback.handleStackMerging(DestackerFallback.java:356)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:205)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:201)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:238)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:216)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:238)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:238)
        at ch.securify.decompiler.DestackerFallback.decompile(DestackerFallback.java:131)
        at ch.securify.decompiler.DecompilerFallback.decompile(DecompilerFallback.java:73)
        at ch.securify.Main.decompileContract(Main.java:302)
        at ch.securify.Main.processHexFile(Main.java:163)
        at ch.securify.Main.processCompilationOutput(Main.java:132)
        at ch.securify.Main.processSolidityFile(Main.java:103)
        at ch.securify.Main.main(Main.java:245)

with

java -jar build/libs/securify-0.1.jar -fs decompile.sol

In findJumpCondition, there's no guarantee that jumpI.getRawInstruction() is not null. I'm looking into how to initialize it properly.

With my current partial and possibly unsound fix, I get a lot of unhandled exceptions that do not make sense. I suspect this is connected to this TODO: https://github.com/eth-sri/securify/blob/master/src/main/java/ch/securify/decompiler/DestackerFallback.java#L478

This could also be related to the original issue.