Bad handling of illegal class names

[samczsun]

I'm back!

In the default verification mode (e.g not -Xverify:all), it looks like the JVM doesn't verify certain class names. Specifically, a descriptor of [I; will be treated as a regular object type (for example, when parsing com/foobar/Name instead of Lcom/foobar/Name;) even though it ends with a semicolon. This means that a NoClassDefFoundError will be thrown at runtime instead of a VerifyError at linktime.

Sample:

.version 49 0 
.class public final a
.super java/lang/Object 

.method public static main : ([Ljava/lang/String;)V 
    .code stack 10 locals 10
    .catch java/lang/NoClassDefFoundError from L1 to L2 using L2
    L1:
        iconst_0
        anewarray [I;
        return
    L2:
        pop
        getstatic java/lang/System out Ljava/io/PrintStream;
        ldc "Hello"
        invokevirtual java/io/PrintStream println (Ljava/lang/String;)V
        return
    .end code
.end method 

.end class 

Krakatau:

Traceback (most recent call last):
  File "decompile.py", line 158, in <module>
    decompileClass(path, targets, args.out, args.skip, magic_throw=args.xmagicthrow)
  File "decompile.py", line 103, in decompileClass
    source = printer.visit(javaclass.generateAST(c, makeGraphCB, skip_errors, add_throws=add_throws))
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\java\javaclass.py", line 67, in generateAST
    method_defs = [_getMethod(m, cb, forbidden_identifiers, skip_errors) for m in methods]
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\java\javaclass.py", line 37, in _getMethod
    graph = cb(method) if method.code is not None else None
  File "decompile.py", line 49, in makeGraph
    s = Krakatau.ssa.ssaFromVerified(m.code, v, opts)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\graph.py", line 717, in ssaFromVerified
    data = blockmaker.BlockMaker(parent, iNodes, inputTypes, returnTypes, code.except_raw, opts=opts)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 127, in __init__
    vals, outslot_norm = self._getInstrLine(node)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 229, in _getInstrLine
    vals = instructionHandlers[instr[0]](self, inslots, iNode)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmakerfuncs.py", line 61, in _anewarray
    tt = parseArrOrClassName(name)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmakerfuncs.py", line 28, in parseArrOrClassName
    vtypes = parseFieldDescriptor(desc, unsynthesize=False)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\verifier\descriptors.py", line 50, in parseFieldDescriptor
    rval = parseFieldDescriptors(desc_str, unsynthesize)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\verifier\descriptors.py", line 29, in parseFieldDescriptors
    raise ValueError('Unrecognized code {} in descriptor'.format(desc_str[0]))
ValueError: Unrecognized code ; in descriptor

Output:

Hello

Edit: swapping anewarray [I; with new I; will cause the following assertion error:

Traceback (most recent call last):
  File "decompile.py", line 158, in <module>
    decompileClass(path, targets, args.out, args.skip, magic_throw=args.xmagicthrow)
  File "decompile.py", line 103, in decompileClass
    source = printer.visit(javaclass.generateAST(c, makeGraphCB, skip_errors, add_throws=add_throws))
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\java\javaclass.py", line 67, in generateAST
    method_defs = [_getMethod(m, cb, forbidden_identifiers, skip_errors) for m in methods]
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\java\javaclass.py", line 37, in _getMethod
    graph = cb(method) if method.code is not None else None
  File "decompile.py", line 49, in makeGraph
    s = Krakatau.ssa.ssaFromVerified(m.code, v, opts)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\graph.py", line 717, in ssaFromVerified
    data = blockmaker.BlockMaker(parent, iNodes, inputTypes, returnTypes, code.except_raw, opts=opts)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 127, in __init__
    vals, outslot_norm = self._getInstrLine(node)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 223, in _getInstrLine
    parent.setObjVarData(ivar, iNode.state.stack[i], initMap)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\graph.py", line 688, in setObjVarData
    tt = objtypes.verifierToSynthetic(vtype2)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\objtypes.py", line 91, in verifierToSynthetic
    assert vtype.tag not in (None, '.address', '.new', '.init')
AssertionError

This variation causes a different error:

.method public static main : ([Ljava/lang/String;)V 
    .code stack 10 locals 10
    .catch [0] from L1 to L2 using L2
    L1:
        new I;
        dup
        invokespecial I; <init> ()V
        aload_0
        ifnonnull L3
        astore_0
        goto L1
    L2:
        pop
        getstatic java/lang/System out Ljava/io/PrintStream;
        ldc "Hello"
        invokevirtual java/io/PrintStream println (Ljava/lang/String;)V
        return
    L3:
        return
    .end code
.end method 

Traceback (most recent call last):
  File "decompile.py", line 158, in <module>
    decompileClass(path, targets, args.out, args.skip, magic_throw=args.xmagicthrow)
  File "decompile.py", line 103, in decompileClass
    source = printer.visit(javaclass.generateAST(c, makeGraphCB, skip_errors, add_throws=add_throws))
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\java\javaclass.py", line 67, in generateAST
    method_defs = [_getMethod(m, cb, forbidden_identifiers, skip_errors) for m in methods]
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\java\javaclass.py", line 37, in _getMethod
    graph = cb(method) if method.code is not None else None
  File "decompile.py", line 49, in makeGraph
    s = Krakatau.ssa.ssaFromVerified(m.code, v, opts)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\graph.py", line 717, in ssaFromVerified
    data = blockmaker.BlockMaker(parent, iNodes, inputTypes, returnTypes, code.except_raw, opts=opts)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 112, in __init__
    self.makeBlock(key)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 327, in makeBlock
    return self.makeBlockWithInslots(key, node.state.locals, node.state.stack)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 319, in makeBlockWithInslots
    newlocals = self.pruneUnused(key, newlocals)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 187, in pruneUnused
    return {i: newlocals[i] for i in used}
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\ssa\blockmaker.py", line 187, in <dictcomp>
    return {i: newlocals[i] for i in used}
KeyError: 0

[Storyyeller]

Nice to have you back. :)

This kind of issue doesn't sound too serious, but I'll try to look into it when I have the time. Glad someone's willing to do the dirty work of finding all the places the JVM forgot to verify stuff.

[samczsun]

Thanks! I also have a completely unrelated question but since I don't have any other ways of contacting you...

Do you happen to know whether Krakatau can handle lots of small exception handlers better, or a few large exception handlers better? For files which are heavily obfuscated with handlers littered all over the place I can either automate splitting them into small individual handlers spanning a single instruction, or merging them into as big a handler as possible. However, I don't know if one has a heavier performance impact than the other.

[Storyyeller]

Krakatau already merges and splits exception handlers during the initial SSA-CFG construction, so it is unlikely to make much difference, as long as the logic is the same. I would expect larger handlers to be faster in most cases, but assuming they simplify to the same SSA-CFG, then the only part that will be affected is the initial bytecode parsing and verification, which is usually dwarfed by the later passes.

[Storyyeller]

I get a linkage error when I try to run your examples. Presumably, this is another place where Java 9 added verification that wasn't verified before. Given this, I don't think it's worth the effort to fix.