Review Request: 8221530: Caller sensitive methods not handling caller = null when invoked by JNI code with no java frames on stack

[Peter Levart]

Hi,

On 3/28/19 9:40 AM, Alan Bateman wrote:
> On 27/03/2019 23:17, Mandy Chung wrote:
>> :
>>
>> The proposed fix is to perform proper access check.  When there is no
>> caller frame, it only allows to access to public members of a public 
>> type
>> in an unconditional exported API package.
>>
> The approach seems reasonable to me and we should, at some point, try 
> to align the other @CS methods with this behavior. As Mandy knows, 
> this is unspecified behavior and we aren't consistent in the JDK on 
> how to handle "no caller frame" case. Some @CS methods will throw NPE 
> if invoked directly, others detect the caller is null and throw or 
> treat it as Object.class.
>
> In the patch, shouldn't slowVerifyAccess check that memberClass is 
> public?

I think it should. If we pretend that the check should be equivalent as 
if performed from an unrelated class in an unrelated module, then the 
check for public member class should be included.

In addition, if access from null caller is granted and it is performed 
to a member in a "concealed" package, there's no warning displayed (the 
further logic in the AccessibleObject is skipped).

What would it look like if AccessibleObject was left intact and only 
Reflection was modified to accommodate for null currentClass - 
caller/accessor. Like the in the following patch:

Index: src/java.base/share/classes/jdk/internal/reflect/Reflection.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/java.base/share/classes/jdk/internal/reflect/Reflection.java 
(revision 54324:9d5c84b0a59862859081e2477f8069a2149c4063)
+++ src/java.base/share/classes/jdk/internal/reflect/Reflection.java 
(revision 54324+:9d5c84b0a598+)
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights 
reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -108,7 +108,10 @@
      /**
       * Verify access to a member and return {@code true} if it is granted.
       *
-     * @param currentClass the class performing the access
+     * @param currentClass the class performing the access or null if
+     *                     access is to be verified for an unknown 
caller/accessor
+     *                     for example if access is performed via JNI 
and there's no
+     *                     Java frame on caller stack (custom JVM 
launchers)
       * @param memberClass the declaring class of the member being accessed
       * @param targetClass the class of target object if accessing instance
       *                    field or method;
@@ -122,6 +125,13 @@
                                               Class<?> targetClass,
                                               int modifiers)
      {
+        Objects.requireNonNull(memberClass);
+
+        if (currentClass == null) {
+            // verify access for an unknown caller/accessor
+            return verifyGeneralMemberAccess(memberClass, modifiers);
+        }
+
          if (currentClass == memberClass) {
              // Always succeeds
              return true;
@@ -201,6 +211,17 @@
          return true;
      }

+    /**
+     * @param memberClass the member's declaring class
+     * @param modifiers the member's modifiers
+     * @return {@code true} if the member can be accessed from anywhere
+     */
+    private static boolean verifyGeneralMemberAccess(Class<?> 
memberClass, int modifiers) {
+        return Modifier.isPublic(getClassAccessFlags(memberClass)) &&
+               Modifier.isPublic(modifiers) &&
+ memberClass.getModule().isExported(memberClass.getPackageName());
+    }
+
      /**
       * Returns {@code true} if memberClass's module exports memberClass's
       * package to currentModule.
@@ -327,6 +348,9 @@
int modifiers)
          throws IllegalAccessException
      {
+        if (currentClass == null)
+            return newIllegalAccessException(memberClass, modifiers);
+
          String currentSuffix = "";
          String memberSuffix = "";
          Module m1 = currentClass.getModule();
@@ -352,6 +376,37 @@
              if (m2.isNamed()) msg += " to " + m1;
          }

+        return new IllegalAccessException(msg);
+    }
+
+    /**
+     * Returns an IllegalAccessException with an exception message where
+     * there is no caller frame.
+     */
+    public static IllegalAccessException 
newIllegalAccessException(Class<?> memberClass,
+ int modifiers)
+        throws IllegalAccessException
+    {
+        String memberSuffix = "";
+        Module m2 = memberClass.getModule();
+        if (m2.isNamed())
+            memberSuffix = " (in " + m2 + ")";
+
+        String memberPackageName = memberClass.getPackageName();
+
+        String msg = "JNI attached native thread (null caller frame) 
cannot access ";
+        if (m2.isExported(memberPackageName)) {
+
+            // module access okay so include the modifiers in the message
+            msg += "a member of " + memberClass + memberSuffix +
+                " with modifiers \"" + Modifier.toString(modifiers) + "\"";
+
+        } else {
+            // module access failed
+            msg += memberClass + memberSuffix+ " because "
+                + m2 + " does not export " + memberPackageName;
+        }
+
          return new IllegalAccessException(msg);
      }



Regards, Peter