RFR(S) 8160987: JDWP ClassType.InvokeMethod doesn't validate class

[Daniel D. Daugherty]

On 9/15/16 9:31 AM, harold seigel wrote:
> Hi,
>
> Please review this small fix for JDK-8160987.  The JDWP InvokeStatic() 
> method was depending on the JNI function that it called to enforce the 
> requirement that the specified method must be a member of the 
> specified class or one of its super classes. But, JNI does not enforce 
> this requirement.  This fix adds code to JDWP to do its own check that 
> the specified method is a member of the specified class or one of its 
> super classes.
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8160987
>
> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8160987/

src/jdk.jdwp.agent/share/native/libjdwp/invoker.c
     Sorry I didn't think of this comment during the pre-review...

     The only "strange" part of this fix is:

     L374:     /* Get the method object from the method's jmethodID. */
     L375:     method_object = JNI_FUNC_PTR(env,ToReflectedMethod)(env,
     L376: clazz,
     L377: method,
     L378: JNI_TRUE /* isStatic */);
     L379:     if (method_object == NULL) {
     L380:         return JVMTI_ERROR_NONE; /* Bad jmethodID ? This will 
be handled elsewhere */
     L381:     }

     Here we are using parameter 'clazz' to find the method_object for
     parameter 'method' so that we can validate that 'clazz' refers to
     method's class or superclass.

     When a bogus 'clazz' value is passed in by a JCK test, the only
     reason that JNI ToReflectedMethod() can still find the right
     method_object is that our (HotSpot) implementation of JNI
     ToReflectedMethod() doesn't really require the 'clazz' parameter
     to find the right method_object. So the 'method_object' that we
     return is the real one which has a 'clazz' field that doesn't
     match the 'clazz' parameter.

     Wow does that twist your head around or what?

     So we're trusting JNI ToReflectedMethod() to return the right
     method_object when we give it a potentially bad 'clazz' value.

     So should we use JNI FromReflectedMethod() to convert the
     method_object back into a jmethodID and verify that jmethodID
     matches the one that we passed to check_methodClass()?

I might be too paranoid here so feel free to say that enough is
enough with this fix.

Thumbs up!

Dan


>
> The fix was tested with the two failing JCK vm/jdwp tests listed in 
> the bug, the JCK Lang, VM, and API tests, the hotspot JTReg tests, the 
> java/lang, java/util and other JTReg tests, the co-located and 
> non-colocated NSK tests, and with the RBT Tier2 tests.
>
> Thanks, Harold
>