From e6175de4828641f1d54809d91be2f7b58e58a05c Mon Sep 17 00:00:00 2001 From: Jayathirth Rao D V Date: Mon, 28 Aug 2023 10:26:45 +0530 Subject: [PATCH 1/2] 8315074: Possible null pointer access in native glass --- .../native-glass/gtk/GlassApplication.cpp | 9 ++++-- .../src/main/native-glass/gtk/GlassTimer.cpp | 14 +++++--- .../src/main/native-glass/gtk/glass_general.h | 6 ++++ .../native-glass/gtk/glass_window_ime.cpp | 15 ++++++++- .../src/main/native-glass/ios/GlassMacros.h | 32 +++++++++++++------ .../main/native-glass/mac/GlassApplication.m | 22 ++++++++----- .../src/main/native-glass/mac/GlassKey.m | 3 ++ .../src/main/native-glass/mac/GlassMacros.h | 32 +++++++++++++------ 8 files changed, 98 insertions(+), 35 deletions(-) diff --git a/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp b/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp index ba56d912961..b4fc90e9ba3 100644 --- a/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp +++ b/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp @@ -262,8 +262,13 @@ JNIEXPORT void JNICALL Java_com_sun_glass_ui_gtk_GtkApplication__1submitForLater (void)obj; RunnableContext* context = (RunnableContext*)malloc(sizeof(RunnableContext)); - context->runnable = env->NewGlobalRef(runnable); - gdk_threads_add_idle_full(G_PRIORITY_HIGH_IDLE + 30, call_runnable, context, NULL); + if (context != NULL) { + context->runnable = env->NewGlobalRef(runnable); + gdk_threads_add_idle_full(G_PRIORITY_HIGH_IDLE + 30, call_runnable, context, NULL); + // we release this context in call_runnable + } else { + fprintf(stderr, "malloc failed in GtkApplication__1submitForLaterInvocatio\n"); + } } /* diff --git a/modules/javafx.graphics/src/main/native-glass/gtk/GlassTimer.cpp b/modules/javafx.graphics/src/main/native-glass/gtk/GlassTimer.cpp index d5154ae6ef3..6aab7d138d8 100644 --- a/modules/javafx.graphics/src/main/native-glass/gtk/GlassTimer.cpp +++ b/modules/javafx.graphics/src/main/native-glass/gtk/GlassTimer.cpp @@ -45,10 +45,16 @@ JNIEXPORT jlong JNICALL Java_com_sun_glass_ui_gtk_GtkTimer__1start (void)obj; RunnableContext* context = (RunnableContext*) malloc(sizeof(RunnableContext)); - context->runnable = env->NewGlobalRef(runnable); - context->flag = 0; - gdk_threads_add_timeout_full(G_PRIORITY_HIGH_IDLE, period, call_runnable_in_timer, context, NULL); - return PTR_TO_JLONG(context); + if (context != NULL) { + context->runnable = env->NewGlobalRef(runnable); + context->flag = 0; + gdk_threads_add_timeout_full(G_PRIORITY_HIGH_IDLE, period, call_runnable_in_timer, context, NULL); + return PTR_TO_JLONG(context); + } else { + // we throw RuntimeException on Java side when we can't + // start the timer + return 0L; + } } /* diff --git a/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.h b/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.h index 3be4a1ee740..e071efc1c27 100644 --- a/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.h +++ b/modules/javafx.graphics/src/main/native-glass/gtk/glass_general.h @@ -96,6 +96,12 @@ struct jni_exception: public std::exception { jstring jmessage; }; +#define SAFE_FREE(PTR) \ + if ((PTR) != NULL) { \ + free(PTR); \ + (PTR) = NULL; \ + } + #define EXCEPTION_OCCURED(env) (check_and_clear_exception(env)) #define CHECK_JNI_EXCEPTION(env) \ diff --git a/modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp b/modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp index 63e816a5221..213c7468076 100644 --- a/modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp +++ b/modules/javafx.graphics/src/main/native-glass/gtk/glass_window_ime.cpp @@ -59,6 +59,12 @@ bool WindowContextBase::im_filter_keypress(GdkEventKey* event) { buffer = (char*)malloc(buf_len * sizeof (char)); } + if (buffer == NULL) { + // dont process the key event + fprintf(stderr, "malloc failed in im_filter_keypress\n"); + return false; + } + KeySym keysym; Status status; XKeyPressedEvent xevent = convert_event(event); @@ -74,7 +80,14 @@ bool WindowContextBase::im_filter_keypress(GdkEventKey* event) { int len = Xutf8LookupString(xim.ic, &xevent, buffer, buf_len - 1, &keysym, &status); if (status == XBufferOverflow) { buf_len = len + 1; - buffer = (char*)realloc(buffer, buf_len * sizeof (char)); + char *tmpBuffer = (char*)realloc(buffer, buf_len * sizeof (char)); + if (tmpBuffer == NULL) { + SAFE_FREE(buffer); + // dont process the key event + fprintf(stderr, "realloc failed in im_filter_keypress\n"); + return false; + } + buffer = tmpBuffer; len = Xutf8LookupString(xim.ic, &xevent, buffer, buf_len - 1, &keysym, &status); } diff --git a/modules/javafx.graphics/src/main/native-glass/ios/GlassMacros.h b/modules/javafx.graphics/src/main/native-glass/ios/GlassMacros.h index fa1f517517b..2cc16845b89 100644 --- a/modules/javafx.graphics/src/main/native-glass/ios/GlassMacros.h +++ b/modules/javafx.graphics/src/main/native-glass/ios/GlassMacros.h @@ -73,22 +73,34 @@ do { GlassThreadData *_GlassThreadData = (GlassThreadData*)pthread_getspecific(GlassThreadDataKey); \ if (_GlassThreadData == NULL) \ { \ - _GlassThreadData = malloc(sizeof(GlassThreadData)); \ - memset(_GlassThreadData, 0x00, sizeof(GlassThreadData)); \ - pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \ + _GlassThreadData = calloc(1, sizeof(GlassThreadData)); \ + if (_GlassThreadData != NULL) \ + { \ + pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \ + } \ + else \ + { \ + fprintf(stderr, "malloc failed in GLASS_POOL_ENTER\n"); \ + } \ } \ - assert(_GlassThreadData->counter >= 0); \ - if (_GlassThreadData->counter++ == 0) \ + if (_GlassThreadData != NULL) \ { \ - _GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \ + assert(_GlassThreadData->counter >= 0); \ + if (_GlassThreadData->counter++ == 0) \ + { \ + _GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \ + } \ } #define GLASS_POOL_EXIT \ - if (--_GlassThreadData->counter == 0) \ + if (_GlassThreadData != NULL) \ { \ - [_GlassThreadData->pool drain]; \ - _GlassThreadData->pool = nil; \ + if (--_GlassThreadData->counter == 0) \ + { \ + [_GlassThreadData->pool drain]; \ + _GlassThreadData->pool = nil; \ + } \ + assert(_GlassThreadData->counter >= 0); \ } \ - assert(_GlassThreadData->counter >= 0); \ } // variations of GLASS_POOL_ENTER/GLASS_POOL_EXIT allowing them to be used accross different calls diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassApplication.m b/modules/javafx.graphics/src/main/native-glass/mac/GlassApplication.m index d85dfcc348b..be65b437322 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassApplication.m +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassApplication.m @@ -362,6 +362,7 @@ - (void)application:(NSApplication *)theApplication openFiles:(NSArray *)filenam LOG("GlassApplication:application:openFiles"); GET_MAIN_JENV; + NSApplicationDelegateReply reply = NSApplicationDelegateReplySuccess; NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init]; { NSUInteger count = [filenames count]; @@ -371,21 +372,26 @@ - (void)application:(NSApplication *)theApplication openFiles:(NSArray *)filenam } jobjectArray files = (*env)->NewObjectArray(env, (jsize)count, stringClass, NULL); GLASS_CHECK_EXCEPTION(env); - for (NSUInteger i=0; iSetObjectArrayElement(env, files, (jsize)i, (*env)->NewStringUTF(env, [file UTF8String])); - GLASS_CHECK_EXCEPTION(env); + NSString *file = [filenames objectAtIndex:i]; + if (file != nil) + { + (*env)->SetObjectArrayElement(env, files, (jsize)i, (*env)->NewStringUTF(env, [file UTF8String])); + GLASS_CHECK_EXCEPTION(env); + } } + (*env)->CallVoidMethod(env, self->jApplication, [GlassHelper ApplicationNotifyOpenFilesMethod], files); + } else { + fprintf(stderr, "NewObjectArray failed in GlassApplication_application\n"); + reply = NSApplicationDelegateReplyFailure; } - (*env)->CallVoidMethod(env, self->jApplication, [GlassHelper ApplicationNotifyOpenFilesMethod], files); } [pool drain]; GLASS_CHECK_EXCEPTION(env); - [theApplication replyToOpenOrPrint:NSApplicationDelegateReplySuccess]; + [theApplication replyToOpenOrPrint:reply]; } - (BOOL)application:(NSApplication *)theApplication openFile:(NSString *)filename diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassKey.m b/modules/javafx.graphics/src/main/native-glass/mac/GlassKey.m index f34e89c2c9b..4c2f2bbf381 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassKey.m +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassKey.m @@ -461,6 +461,9 @@ jcharArray GetJavaKeyChars(JNIEnv *env, NSEvent *event) jchar jc[16]; [chars getCharacters:jc range:NSMakeRange(0, [chars length])]; jcharArray jChars = (*env)->NewCharArray(env, (jsize)[chars length]); + if (jChars == NULL) { + return NULL; + } (*env)->SetCharArrayRegion(env, jChars, 0, (jsize)[chars length], jc); GLASS_CHECK_EXCEPTION(env); return jChars; diff --git a/modules/javafx.graphics/src/main/native-glass/mac/GlassMacros.h b/modules/javafx.graphics/src/main/native-glass/mac/GlassMacros.h index 9c8761430a0..5ec7ac8a480 100644 --- a/modules/javafx.graphics/src/main/native-glass/mac/GlassMacros.h +++ b/modules/javafx.graphics/src/main/native-glass/mac/GlassMacros.h @@ -177,22 +177,34 @@ do { GlassThreadData *_GlassThreadData = (GlassThreadData*)pthread_getspecific(GlassThreadDataKey); \ if (_GlassThreadData == NULL) \ { \ - _GlassThreadData = malloc(sizeof(GlassThreadData)); \ - memset(_GlassThreadData, 0x00, sizeof(GlassThreadData)); \ - pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \ + _GlassThreadData = calloc(1, sizeof(GlassThreadData)); \ + if (_GlassThreadData != NULL) \ + { \ + pthread_setspecific(GlassThreadDataKey, _GlassThreadData); \ + } \ + else \ + { \ + fprintf(stderr, "malloc failed in GLASS_POOL_ENTER\n"); \ + } \ } \ - assert(_GlassThreadData->counter >= 0); \ - if (_GlassThreadData->counter++ == 0) \ + if (_GlassThreadData != NULL) \ { \ - _GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \ + assert(_GlassThreadData->counter >= 0); \ + if (_GlassThreadData->counter++ == 0) \ + { \ + _GlassThreadData->pool = [[NSAutoreleasePool alloc] init]; \ + } \ } #define GLASS_POOL_EXIT \ - if (--_GlassThreadData->counter == 0) \ + if (_GlassThreadData != NULL) \ { \ - [_GlassThreadData->pool drain]; \ - _GlassThreadData->pool = nil; \ + if (--_GlassThreadData->counter == 0) \ + { \ + [_GlassThreadData->pool drain]; \ + _GlassThreadData->pool = nil; \ + } \ + assert(_GlassThreadData->counter >= 0); \ } \ - assert(_GlassThreadData->counter >= 0); \ } // variations of GLASS_POOL_ENTER/GLASS_POOL_EXIT allowing them to be used accross different calls From 68e3b196468bc2e51407f8fd1032c073aaa63dc0 Mon Sep 17 00:00:00 2001 From: Jayathirth Rao D V Date: Wed, 30 Aug 2023 21:13:25 +0530 Subject: [PATCH 2/2] Fix typo --- .../src/main/native-glass/gtk/GlassApplication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp b/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp index b4fc90e9ba3..4cf2a20b5d3 100644 --- a/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp +++ b/modules/javafx.graphics/src/main/native-glass/gtk/GlassApplication.cpp @@ -267,7 +267,7 @@ JNIEXPORT void JNICALL Java_com_sun_glass_ui_gtk_GtkApplication__1submitForLater gdk_threads_add_idle_full(G_PRIORITY_HIGH_IDLE + 30, call_runnable, context, NULL); // we release this context in call_runnable } else { - fprintf(stderr, "malloc failed in GtkApplication__1submitForLaterInvocatio\n"); + fprintf(stderr, "malloc failed in GtkApplication__1submitForLaterInvocation\n"); } }