A Bug involving MethodHandles, Nestmates, Reflection and @CallerSensitive

[David Holmes]

Hi Johannes,

I've filed this bug report for you:

https://bugs.openjdk.java.net/browse/JDK-8257874

Thanks,
David

On 8/12/2020 11:20 am, 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