On Thu, 6 Apr 2023 16:13:38 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line
>> 284:
>>
>>> 282: return
>>> type.getDeclaredAnnotation(WrapperInstance.class);
>>> 283: }
>>> 284: }) : type.getDeclaredAnnotation(WrapperInstance.class);
>>
>> This may introduce a security vulnerability:
>>
>>
>> @AnnotationTest.ClassHolder(sun.misc.Unsafe.class)
>> public class AnnotationTest {
>>
>> @Target(ElementType.TYPE)
>> @Retention(RetentionPolicy.RUNTIME)
>> @interface ClassHolder {
>> Class<?> value();
>> }
>> public static void main(String[] args) throws PrivilegedActionException {
>> MethodHandleProxies.isWrapperInstance(new AnnotationTest());
>>
>> System.out.println(AnnotationTest.class.getDeclaredAnnotation(ClassHolder.class).value());
>> }
>> }
>>
>>
>> Don't parse annotations in a privileged context.
>
> I'm not sure how the example shows that this is a security vulnerability? It
> still works fine without the call to `isWrapperInstance` (even if you switch
> to using jdk.internal.misc.Unsafe.class, although that also requires
> `--add-exports` when compiling)
Sorry, you are supposed to run it with an installed `SecurityManager` of
course.
With an installed `SecurityManager` you should not be able to access classes in
`sun.misc`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1160006762
