A Bug involving MethodHandles, Nestmates, Reflection and @CallerSensitive

[Johannes Kuhn]

On 09-Dec-20 1:16, Mandy Chung wrote:
> On 12/8/20 5:07 AM, Johannes Kuhn wrote:
>>
>> Not sure if I read this correctly as "please share some example of 
>> code that tries to do that" or "please share code that you write to 
>> fix that".
>> So I do both.
>>
>
> I was curious to find out what old software attempts to change static 
> final fields via reflection and why they do that.   This leads to 
> various unpredictable issues.   Looks like you want to get old 
> software to work and have to hack their static final fields!!
Just do a quick search for "NoSuchFieldException modifiers". A lot of 
code does it, my impression is that often someone tried to be clever - 
just because they can.
>
>> Setting static final fields does not work [1]. It probably never 
>> really did. It usually seems to work - but there is no guarantee that 
>> it actually does (like undefined behavior).
>
> Field::setXXX will throw IAE on static final fields and non-static 
> fields declared on a record or hidden class too.
The example works with Java 8 and 11 - it "tricks" the JIT into constant 
folding a static final field, so the subsequent change is not reflected.
>
>> For the nestmate security consideration, the following invariants 
>> should already hold:
>> * To call a @CallerSensitive method, the Lookup needs to have full 
>> privilege access (Lookup.hasFullPrivilegeAccess())
>> -> Injected Invokers are only created for full privilege lookups.
>> * The injected invoker is in the same runtime package and has the 
>> same protection domain.
>> * It is already possible to obtain a Lookup for the injected invoker 
>> by calling MethodHandles.lookup() through a MethodHandle (yikes) [5].
>
> I am concerned with this.  I am inclined to say we need to fix 
> JDK-8013527 if we fix this issue.
>
>>
>> This means, we only have to consider what additional privileges the 
>> injected invoker gets if it is also a nestmate of the lookup class.
>> I don't see any problem with that, as you already have a full 
>> privilege lookup when the injected invoker is created.
>>
>> - Johannes
>>
>> PS.: JDK-8013527 is mildly related - as the @CallerSensitive methods 
>> in java.lang.reflect are "hyper-sensitive".
>
> This fix will get JDK-8013527 passed the IAE but the lookup class of 
> the resulting Lookup object is the hidden class which is still incorrect.
>
> Mandy
A quick history about JDK-8013527 (and JDK-8207834 - which has the same 
root cause, but I wrote an comment on that):

In Java 8, the injected invoker class had a name starting with 
"java.lang.invoke.". Creating a lookup for a class that starts with that 
name or binding the caller for such a class is explicitly blocked.

In Java 9 the name of the injected invoker has changed - from this point 
on, the name of the injected invoker did depend on the package of the 
lookup class / caller.
**It is already possible** to obtain a Lookup for the injected invoker 
by calling MethodHandles.lookup() through a MethodHandle.
It doesn't throw an IAE anymore. **Since Java 9.**

There are a lot of things to consider when trying to fix JDK-8013527.
The first problem is: How do you determine the original class?
It doesn't work with just using the nest host - as this might be 
different from the original class.
Using the name might also not work - as the original class could be a 
hidden class itself.

So, the only real solution is to attach that information (a reference to 
the lookup class) to the injected invoker.
Then we have to detect that the target class is an injected invoker - 
and replace it with the actual lookup class.

And this comes with some cost - either the job is done by 
Reflection.getCallerClass() - which would fix any other problem of that 
kind - or any hyper-sensitive @CallerSensitive method has to do this on 
it's own. (Luckily there are only a handful @CallerSensitive methods - 
and only a few are hyper-sensitive.)

Unfortunately, I don't know enough about hotspot to determine if it is 
possible to change the "owner class" of some code. Also, garbage 
collection...
I don't know enough about those systems. I only look at the stuff from 
the Java / bytecode side.

In the end, I'm not sure if fixing l.lookupClass() == ((Lookup) 
l.findStatic(MethodHandles.class, "lookup", 
MethodType.methodType(Lookup.class)).invokeExact()).lookupClass() is 
worth that effort.

- Johannes