<AWT Dev> [15] Review request for 8242498: Invalid "sun.awt.TimedWindowEvent" object leads to JVM crash

[Anton Litvinov]

Hi Sergey,

Thank you for review of this fix. To be able to reproduce this bug it is 
necessary to do such things with JDK which are not mentioned or 
recommended in Java SE API Specification, so this bug is possible only 
under very specific abnormal conditions requiring deliberate developer's 
intervention. So in normal scenarios of usage of JDK this bug does not 
happen.

I think that it is better not to change current well established old 
behavior in the method "AwtWindow::SendWindowEvent(jint, HWND, jint, 
jint)" - what is first to get "jobject" from the original "opposite" 
HWND and try to use it for construction of "TimedWindowEvent". I was 
trying to make a fix which will just extend the code and will address 
just this very unreal and very narrow scenario without running the risk 
of affecting or breaking wide range of other normal scenarios under 
which this code was working well from the moment of its creation. If we 
assign parent HWND to the "opposite" from the very beginning, then we 
will change the currently existing behavior in favor of addressing this 
almost unreal bug and may for some cases, which we do not know yet, 
start using parent HWND instead of a valid original "opposite" HWND 
whose target will be instance of "java.awt.Window".

In my opinion it is better not to omit first attempt to use original 
"opposite" with related to it "jobject" instance.

Thank you,
Anton

On 14/04/2020 16:41, Sergey Bylokhov wrote:
> Hi, Anton.
>
> Probably it is possible to simplify the code a little bit:
>
> Can we just replace initial "opposite" by the top level HWND?
>
> + opposite = AwtComponent::GetTopLevelParentForWindow(opposite);
>   AwtComponent *awtOpposite = AwtComponent::GetComponent(opposite);
>
> and add only one check
>
>   jOpposite = awtOpposite->GetTarget(env);
> + if ((jOpposite != NULL) &&
> +   !env->IsInstanceOf(jOpposite, windowCls)) {
> +       env->DeleteLocalRef(jOpposite);
> +       jOpposite = NULL;
> +  }
>
> On 4/10/20 1:32 pm, Anton Litvinov wrote:
>> Hello,
>>
>> Could you please review the following fix for the bug.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8242498
>> Webrev: http://cr.openjdk.java.net/~alitvinov/8242498/jdk15/webrev.00
>>
>> The bug is the JVM crash, which occurs because a not existing method 
>> is called on a Java object which is not an instance of the expected 
>> Java class that has such a method. Such discrepancy of the expected 
>> type and the type in runtime is possible, because the Java object, 
>> whose field value is set to the instance of the not expected Java 
>> class, is instantiated by AWT native code through JNI invocation. 
>> Since JNI does not validate arguments passed to Java class 
>> constructor and since AWT native code does not validate arguments 
>> prior to invoking Java class constructor through JNI, such invalid 
>> object is created.
>>
>> REASON OF THE CRASH:
>> The fact that in the method 
>> "java.awt.DefaultKeyboardFocusManager.dispatchEvent(AWTEvent)" in the 
>> case "WindowEvent.WINDOW_LOST_FOCUS" of switch operator the variable 
>> defined by the expression "Window oppositeWindow = 
>> we.getOppositeWindow();" in runtime is instance of 
>> "java.awt.Component" class instead of "java.awt.Window" class. The 
>> crash occurs during attempt to call the method 
>> "java.awt.Window.getTemporaryLostComponent()" on the object 
>> "oppositeWindow" which in runtime is "Component" instead of the 
>> expected "Window" object, and since the method 
>> "getTemporaryLostComponent()" does not exist in "java.awt.Component" 
>> class JVM cannot find this method and initiates the crash.
>>
>> ROOT CAUSE OF THE BUG:
>> Transfer of the object of the incompatible type "java.awt.Component" 
>> instead of an object of "java.awt.Window" type as "opposite" argument 
>> to the constructor "TimedWindowEvent(Window source, int id, Window 
>> opposite, int oldState, int newState, long time)" of the class 
>> "sun.awt.TimedWindowEvent" through JNI invocation. This JNI 
>> invocation occurs in the C++ class method 
>> "AwtWindow::SendWindowEvent(jint, HWND, jint, jint)" in the file 
>> "src/java.desktop/windows/native/libawt/windows/awt_Window.cpp". The 
>> exact expression creating the instance of Java class 
>> "TimedWindowEvent" with the invalid value of the field "opposite" is 
>> following:
>>
>> jobject event = env->NewObject(wClassEvent, wEventInitMID, target, id,
>>          jOpposite, oldState, newState, ::JVM_CurrentTimeMillis(NULL, 
>> 0));
>>
>> THE FIX:
>> The fix changes "AwtWindow::SendWindowEvent(jint, HWND, jint, jint)" 
>> method in the file "awt_Window.cpp" to introduce the code which 
>> verifies that the Java object "jOpposite" is really instance of the 
>> class "java.awt.Window", and if it is not then the fix tries to get 
>> Java object corresponding to parent window of the original "opposite" 
>> HWND. And if this parent window object also is not instance of 
>> "java.awt.Window" class, then NULL value is passed to the constructor 
>> of "TimedWindowEvent" class.
>>
>> Thank you,
>> Anton
>
>