RFR: 6983726: remove Proxy from MethodHandleProxies.asInterfaceInstance SAM conversion [v8]

Chen Liang liach at openjdk.org
Fri Apr 7 00:14:49 UTC 2023


On Thu, 6 Apr 2023 19:33:51 GMT, Johannes Kuhn <jkuhn at openjdk.org> wrote:

>> This is good work.    This needs some careful study on the security aspect.  I see the proxy class is in the same module of the interface, is non-public, and it has a no-arg non-public constructor. 
>> 
>> Defining the proxy class in the same module would grant this class reflective access to any class in this module.  This seems a concern if the target method handle should not have access to the class in that module.
>> 
>> W.r.t. the no-arg constructor,  I would consider another level of defense to make the constructor to take `Lookup` of itself and throw an IAE if the lookup class is not itself or it does not have `ORIGINAL` access.
>
>> This seems a concern if the target method handle should not have access to the class in that module.
> 
> MethodHandle access is checked when the MethodHandle is created.  
> For `@CallerSensitive` methods, the MethodHandle is additionally bound to the lookup class.  
> Also see [`java.lang.invoke.ConstantBootstraps.invoke`](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/lang/invoke/ConstantBootstraps.html#invoke%28java.lang.invoke.MethodHandles.Lookup,java.lang.String,java.lang.Class,java.lang.invoke.MethodHandle,java.lang.Object...%29). If calling an arbitrary MethodHandle from a module could be a problem, then that is an easier target.
> 
>> W.r.t. the no-arg constructor, I would consider another level of defense to make the constructor to take Lookup of itself and throw an IAE if the lookup class is not itself or it does not have ORIGINAL access.
> 
> Without that defense you can create a new instance (if you have a reference to the hidden class) that does exactly the same thing as the original instance.  
> Other than `getClass` on the original object, `StackWalker` comes to mind as a way to obtain the class reference.

This patch is ready for review. WrapperInstance change was to address the annotation security concerns raised by @DasBrain. If there's more security implications with null classloader, I will swiftly update this patch as well.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/13197#issuecomment-1499781366


More information about the core-libs-dev mailing list