JDK-8257874 : MethodHandle injected invoker doesn't have necessary private access
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.lang.invoke
  • Affected Version: 15
  • Priority: P3
  • Status: Resolved
  • Resolution: Duplicate
  • Submitted: 2020-12-08
  • Updated: 2021-10-28
  • Resolved: 2021-10-28
Related Reports
Blocks :  
Duplicate :  
Description
Johannes Kuhn wrote:

Let's start with the reproducer:

    public class NestmateBug {
        private static int field = 0;
        public static void main(String[] args) throws Throwable {
            Lookup l = MethodHandles.lookup();
            Field f = NestmateBug.class.getDeclaredField("field");
            MethodHandle mh = l.findVirtual(Field.class, "setInt", methodType(void.class, Object.class, int.class));
            int newValue = 5;
            mh.invokeExact(f, (Object) null, newValue);
        }
    }

This throws a IAE in the last line:

class test.se15.NestmateBug$$InjectedInvoker/0x0000000800bb5840 (in module test.se15) cannot access a member of class test.se15.NestmateBug (in module test.se15) with modifiers "private static"
    at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:385)
    at java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:693)
    at java.base/java.lang.reflect.Field.checkAccess(Field.java:1096)
    at java.base/java.lang.reflect.Field.setInt(Field.java:976)
    at test.se15/test.se15.NestmateBug.main(NestmateBug.java:17)

The reason for this behaviour is:
* Field.setInt (without setAccessible) requires that the caller class is in the same nest for private members.
* Field.setInt is @CallerSensitive
* MethodHandles will bind to the caller by injecting an invoker for @CallerSensitive methods.
* The injected invoker is NOT a nestmate of the original lookup class.
* The access check of Field.setInt fails - as the injected invoker is now the caller.

This is important because:
* Some old software loves to set static final fields through reflection.
* To do that, it usually hacks into Field.modifiers. Which is filtered iirc since Java 12.
* I write Java agents to fix such things - removing final from static fields, and intercept Class.getDeclaredField and Field.setInt by replacing it with an invokedynamic instruction.
* The original target is passed as a MethodHandle bootstrap argument. In the case of Field.setInt, it is guarded with a MethodHandles.guardWithTest() - calling the original target if the Field is not my canary.

Suggested fix:
* Make the injected invoker a nestmate of lookup class.

I did prepare a fix for that [1], but AFAIK the bug first needs to be filled in the bug tracker.
And I don't have an account.

- Johannes


[1]: https://github.com/DasBrain/jdk/commit/3c4bb20c8e4cd9086e934128e5cb085a5cfbdb94
Comments
The implementation of JEP 416 defines a new calling sequence for caller-sensitive methods in addition to a new @CallerSensitiveAdapter annotation (which is intended for auditing purpose and not for runtime use). The core reflection and method handle will respect this new calling sequence. The adapter method is only needed for the caller-sensitive methods that require an exact caller class correctness. `java.lang.invoke.MethodHandles::lookup` and `ClassLoader::registerAsParallelCapable()` are the only two methods in the JDK that need the exact caller class. All other caller-sensitive methods use the caller's class for access checks or security permission checks, i.e., based on its runtime package, defining loader, or protection domain. Hence, the adapter method is optional.
28-10-2021

JDK-8013527 will leak a lookup on the injected invoker class if this issue is fixed. So JDK-8013527 must be fixed properly with thorough security assessment before this issue is fixed. See https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-December/072202.html
09-12-2020