On Thu, 6 Apr 2023 15:57:33 GMT, Johannes Kuhn <[email protected]> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Whitespace, cleanup, rename benchmarks to be informative
>
> 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)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1160005209
