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

[Mandy Chung]

On 3/28/19 7:48 AM, Peter Levart wrote:
> 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:
>

I intentionally do not change Reflection::verifyMemberAccess to accept 
null caller and see
the patch:

125 Objects.requireNonNull(currentClass);
126 Objects.requireNonNull(memberClass);

Although it is a reasonable default for a null caller to allow access 
public member in a
public type of an unconditional exported API package, the caller should 
carefully consider
how it behaves in this edge case.    This forces the caller to address 
it properly rather
than falling through.  I'm okay to add a new 
Reflection::verifyPublicMemberAccess
which can be called by other if appropriate.

Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221530/webrev.01

Mandy