A Bug involving MethodHandles, Nestmates, Reflection and @CallerSensitive

[Mandy Chung]

Hi Johannes,

There has been a couple of the prototypes regarding @CS methods (Alan 
did one and I did another) in the context of JDK-6824466. There are lots 
of security consideration regarding @CS methods. You are welcome to go 
deeper in that area.  If you are looking for starter bugs to fix, that 
won't be a quick patch.

I also came up with a patch for JDK-8013527 when working on 
JDK-6824466.  It's buried in 
https://github.com/openjdk/jdk/compare/master...mlchung:method-invoke. I 
will extract that fix and post a PR on my proposed fix.

Mandy

On 1/16/21 10:07 AM, Johannes Kuhn wrote:
> After digging though the JBS, I found this comment from John Rose[1].
>
> I like the general idea, but I don't think it's necessary to use a 
> native method as stub. Instead it could be written like this:
>
> class Class {
>   @CallerSensitive
>   @ForceInline
>   public static Class<?> forName(String name) {
>       return forName(name, Reflection.getCallerClass());
>   }
>   private static Class<?> forName(String name, Class<?> caller) {
>       // original implementation
>   }
> }
>
> By doing this, binding to the caller could be done by returning a 
> MethodHandle that actually calls the private method - with the lookup 
> class injected as argument (MethodHandles.insertArguments).
>
> Problem are methods that could be virtually invoked 
> (getContextClassLoader). Therefore it might be useful to keep the old 
> binding logic around. It also reduces the number of places where this 
> change has to be done - it's only required for the 
> hyper- at CallerSensitive methods.
>
> I will try to write a prototype that demonstrates that this approach 
> is feasible.
>
> - Johannes
>
> [1]: 
> https://bugs.openjdk.java.net/browse/JDK-8020968?focusedCommentId=13611844&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13611844
>
> On 09-Dec-20 21:09, Johannes Kuhn wrote:
>> On 09-Dec-20 19:44, Mandy Chung wrote:
>>>
>>>
>>> On 12/8/20 6:02 PM, Johannes Kuhn wrote:
>>>> There are a lot of things to consider when trying to fix JDK-8013527.
>>>
>>> Exactly in particular security implication!  What is clear is that 
>>> the expected lookup class should not be the injected class.   The 
>>> key message here is that we can't fix JDK-8257874 until we fix 
>>> JDK-8013527 unfortunately.
>>>
>>> Mandy
>>>
>> Yeah, if JDK-8013527 is fixed it might fix JDK-8257874 as a byproduct.
>> If Lookup.lookup() can determine the original caller, then 
>> Field.set*/Method.invoke could do the same.
>> Special care has to be taken that no other class could spoof such an 
>> injected invoker.
>>
>> Too complicated for me :). JDK-8013527 needs a sound design to 
>> approach fixing it IMHO.
>>
>> - Johannes
>>