From 8bb86a9d34a886efc72ebb5a838964fd1012bfcb Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 26 Jan 2021 17:25:06 +0000 Subject: [PATCH] fix: handle remaining JNI calls error handling --- CHANGELOG.md | 1 + .../src/main/jni/bugsnag.c | 272 ++++++++++++------ .../src/main/jni/bugsnag_ndk.c | 28 +- .../src/main/jni/metadata.c | 190 ++++++++---- .../src/main/jni/safejni.c | 103 +++++++ .../src/main/jni/safejni.h | 79 +++-- 6 files changed, 484 insertions(+), 189 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 100f40babe..1fbf874127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Check internal JNI calls for pending exceptions and no-op [#1088](https://github.com/bugsnag/bugsnag-android/pull/1088) [#1091](https://github.com/bugsnag/bugsnag-android/pull/1091) + [#1092](https://github.com/bugsnag/bugsnag-android/pull/1092) * Add global metadata to ANR error reports [#1095](https://github.com/bugsnag/bugsnag-android/pull/1095) diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c index e7346829cb..1a4efbe34d 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c @@ -54,12 +54,14 @@ jfieldID bsg_parse_jseverity(JNIEnv *env, bugsnag_severity severity, jclass severity_class) { const char *severity_sig = "Lcom/bugsnag/android/Severity;"; if (severity == BSG_SEVERITY_ERR) { - return (*env)->GetStaticFieldID(env, severity_class, "ERROR", severity_sig); + return bsg_safe_get_static_field_id(env, severity_class, "ERROR", + severity_sig); } else if (severity == BSG_SEVERITY_WARN) { - return (*env)->GetStaticFieldID(env, severity_class, "WARNING", - severity_sig); + return bsg_safe_get_static_field_id(env, severity_class, "WARNING", + severity_sig); } else { - return (*env)->GetStaticFieldID(env, severity_class, "INFO", severity_sig); + return bsg_safe_get_static_field_id(env, severity_class, "INFO", + severity_sig); } } @@ -70,153 +72,211 @@ void bsg_release_byte_ary(JNIEnv *env, jbyteArray array, char *original_text) { } } +void bsg_populate_notify_stacktrace(JNIEnv *env, bugsnag_stackframe *stacktrace, + ssize_t frame_count, jclass trace_class, + jmethodID trace_constructor, + jobjectArray trace) { + for (int i = 0; i < frame_count; i++) { + bugsnag_stackframe frame = stacktrace[i]; + + // create Java string objects for class/filename/method + jstring class = bsg_safe_new_string_utf(env, ""); + if (class == NULL) { + goto exit; + } + + jstring filename = bsg_safe_new_string_utf(env, frame.filename); + jstring method; + if (strlen(frame.method) == 0) { + char *frame_address = malloc(sizeof(char) * 32); + sprintf(frame_address, "0x%lx", (unsigned long)frame.frame_address); + method = bsg_safe_new_string_utf(env, frame_address); + free(frame_address); + } else { + method = bsg_safe_new_string_utf(env, frame.method); + } + + // create StackTraceElement object + jobject jframe = + bsg_safe_new_object(env, trace_class, trace_constructor, class, method, + filename, frame.line_number); + if (jframe == NULL) { + goto exit; + } + + bsg_safe_set_object_array_element(env, trace, i, jframe); + goto exit; + + exit: + (*env)->DeleteLocalRef(env, filename); + (*env)->DeleteLocalRef(env, class); + (*env)->DeleteLocalRef(env, method); + } +} + void bugsnag_notify_env(JNIEnv *env, char *name, char *message, bugsnag_severity severity) { + jclass interface_class = NULL; + jmethodID notify_method = NULL; + jclass trace_class = NULL; + jclass severity_class = NULL; + jmethodID trace_constructor = NULL; + jobjectArray trace = NULL; + jfieldID severity_field = NULL; + jobject jseverity = NULL; + jbyteArray jname = NULL; + jbyteArray jmessage = NULL; + bugsnag_stackframe stacktrace[BUGSNAG_FRAMES_MAX]; ssize_t frame_count = bsg_unwind_stack(bsg_configured_unwind_style(), stacktrace, NULL, NULL); // lookup com/bugsnag/android/NativeInterface - jclass interface_class = + interface_class = bsg_safe_find_class(env, "com/bugsnag/android/NativeInterface"); if (interface_class == NULL) { - return; + goto exit; } // lookup NativeInterface.notify() - jmethodID notify_method = bsg_safe_get_static_method_id( + notify_method = bsg_safe_get_static_method_id( env, interface_class, "notify", "([B[BLcom/bugsnag/android/Severity;[Ljava/lang/StackTraceElement;)V"); if (notify_method == NULL) { - return; + goto exit; } // lookup java/lang/StackTraceElement - jclass trace_class = bsg_safe_find_class(env, "java/lang/StackTraceElement"); + trace_class = bsg_safe_find_class(env, "java/lang/StackTraceElement"); if (trace_class == NULL) { - return; + goto exit; } // lookup com/bugsnag/android/Severity - jclass severity_class = - bsg_safe_find_class(env, "com/bugsnag/android/Severity"); + severity_class = bsg_safe_find_class(env, "com/bugsnag/android/Severity"); if (severity_class == NULL) { - return; + goto exit; } // lookup StackTraceElement constructor - jmethodID trace_constructor = bsg_safe_get_method_id( + trace_constructor = bsg_safe_get_method_id( env, trace_class, "", "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;I)V"); if (trace_constructor == NULL) { - return; + goto exit; } // create StackTraceElement array - jobjectArray trace = bsg_safe_new_object_array(env, frame_count, trace_class); + trace = bsg_safe_new_object_array(env, frame_count, trace_class); if (trace == NULL) { - return; + goto exit; } - for (int i = 0; i < frame_count; i++) { - bugsnag_stackframe frame = stacktrace[i]; + // populate stacktrace object + bsg_populate_notify_stacktrace(env, stacktrace, frame_count, trace_class, + trace_constructor, trace); - // create Java string objects for class/filename/method - jstring class = bsg_safe_new_string_utf(env, ""); - if (class == NULL) { - continue; - } - - jstring filename = bsg_safe_new_string_utf(env, frame.filename); - jstring method; - if (strlen(frame.method) == 0) { - char *frame_address = malloc(sizeof(char) * 32); - sprintf(frame_address, "0x%lx", (unsigned long)frame.frame_address); - method = bsg_safe_new_string_utf(env, frame_address); - free(frame_address); - } else { - method = bsg_safe_new_string_utf(env, frame.method); - } - jobject jframe = - (*env)->NewObject(env, trace_class, trace_constructor, class, method, - filename, frame.line_number); - - bsg_safe_set_object_array_element(env, trace, i, jframe); - (*env)->DeleteLocalRef(env, filename); - (*env)->DeleteLocalRef(env, class); - (*env)->DeleteLocalRef(env, method); + // get the severity field + severity_field = bsg_parse_jseverity(env, severity, severity_class); + if (severity_field == NULL) { + goto exit; + } + // get the error severity object + jseverity = + bsg_safe_get_static_object_field(env, severity_class, severity_field); + if (jseverity == NULL) { + goto exit; } - // Create a severity Error - jobject jseverity = (*env)->GetStaticObjectField( - env, severity_class, bsg_parse_jseverity(env, severity, severity_class)); - - jbyteArray jname = bsg_byte_ary_from_string(env, name); - jbyteArray jmessage = bsg_byte_ary_from_string(env, message); + jname = bsg_byte_ary_from_string(env, name); + jmessage = bsg_byte_ary_from_string(env, message); // set application's binary arch bugsnag_set_binary_arch(env); - (*env)->CallStaticVoidMethod(env, interface_class, notify_method, jname, - jmessage, jseverity, trace); + bsg_safe_call_static_void_method(env, interface_class, notify_method, jname, + jmessage, jseverity, trace); - bsg_release_byte_ary(env, jname, name); - bsg_release_byte_ary(env, jmessage, message); + goto exit; + +exit: + if (jname != NULL) { + bsg_release_byte_ary(env, jname, name); + } + if (jmessage != NULL) { + bsg_release_byte_ary(env, jmessage, message); + } (*env)->DeleteLocalRef(env, jname); (*env)->DeleteLocalRef(env, jmessage); + (*env)->DeleteLocalRef(env, interface_class); + (*env)->DeleteLocalRef(env, notify_method); (*env)->DeleteLocalRef(env, trace_class); - (*env)->DeleteLocalRef(env, trace); (*env)->DeleteLocalRef(env, severity_class); + (*env)->DeleteLocalRef(env, trace_constructor); + (*env)->DeleteLocalRef(env, trace); + (*env)->DeleteLocalRef(env, severity_field); (*env)->DeleteLocalRef(env, jseverity); - (*env)->DeleteLocalRef(env, interface_class); } void bugsnag_set_binary_arch(JNIEnv *env) { + jclass interface_class = NULL; + jmethodID set_arch_method = NULL; + jstring arch = NULL; + // lookup com/bugsnag/android/NativeInterface - jclass interface_class = + interface_class = bsg_safe_find_class(env, "com/bugsnag/android/NativeInterface"); if (interface_class == NULL) { - return; + goto exit; } // lookup NativeInterface.setBinaryArch() - jmethodID set_arch_method = bsg_safe_get_static_method_id( + set_arch_method = bsg_safe_get_static_method_id( env, interface_class, "setBinaryArch", "(Ljava/lang/String;)V"); if (set_arch_method == NULL) { - return; + goto exit; } // call NativeInterface.setBinaryArch() - jstring arch = bsg_safe_new_string_utf(env, bsg_binary_arch()); + arch = bsg_safe_new_string_utf(env, bsg_binary_arch()); if (arch != NULL) { - (*env)->CallStaticVoidMethod(env, interface_class, set_arch_method, arch); + bsg_safe_call_static_void_method(env, interface_class, set_arch_method, + arch); } + + goto exit; + +exit: (*env)->DeleteLocalRef(env, arch); + (*env)->DeleteLocalRef(env, set_arch_method); (*env)->DeleteLocalRef(env, interface_class); } void bugsnag_set_user_env(JNIEnv *env, char *id, char *email, char *name) { // lookup com/bugsnag/android/NativeInterface - jclass interface_class = + jclass interface_class = NULL; + jmethodID set_user_method = NULL; + + interface_class = bsg_safe_find_class(env, "com/bugsnag/android/NativeInterface"); if (interface_class == NULL) { - return; + goto exit; } // lookup NativeInterface.setUser() - jmethodID set_user_method = bsg_safe_get_static_method_id( - env, interface_class, "setUser", "([B[B[B)V"); + set_user_method = bsg_safe_get_static_method_id(env, interface_class, + "setUser", "([B[B[B)V"); if (set_user_method == NULL) { - return; + goto exit; } jbyteArray jid = bsg_byte_ary_from_string(env, id); jbyteArray jemail = bsg_byte_ary_from_string(env, email); jbyteArray jname = bsg_byte_ary_from_string(env, name); - (*env)->CallStaticVoidMethod(env, interface_class, set_user_method, jid, - jemail, jname); + bsg_safe_call_static_void_method(env, interface_class, set_user_method, jid, + jemail, jname); bsg_release_byte_ary(env, jid, id); bsg_release_byte_ary(env, jemail, email); @@ -225,64 +285,90 @@ void bugsnag_set_user_env(JNIEnv *env, char *id, char *email, char *name) { (*env)->DeleteLocalRef(env, jid); (*env)->DeleteLocalRef(env, jemail); (*env)->DeleteLocalRef(env, jname); + + goto exit; + +exit: (*env)->DeleteLocalRef(env, interface_class); + (*env)->DeleteLocalRef(env, set_user_method); } jfieldID bsg_parse_jcrumb_type(JNIEnv *env, bugsnag_breadcrumb_type type, jclass type_class) { const char *type_sig = "Lcom/bugsnag/android/BreadcrumbType;"; if (type == BSG_CRUMB_USER) { - return (*env)->GetStaticFieldID(env, type_class, "USER", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "USER", type_sig); } else if (type == BSG_CRUMB_ERROR) { - return (*env)->GetStaticFieldID(env, type_class, "ERROR", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "ERROR", type_sig); } else if (type == BSG_CRUMB_LOG) { - return (*env)->GetStaticFieldID(env, type_class, "LOG", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "LOG", type_sig); } else if (type == BSG_CRUMB_NAVIGATION) { - return (*env)->GetStaticFieldID(env, type_class, "NAVIGATION", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "NAVIGATION", + type_sig); } else if (type == BSG_CRUMB_PROCESS) { - return (*env)->GetStaticFieldID(env, type_class, "PROCESS", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "PROCESS", type_sig); } else if (type == BSG_CRUMB_REQUEST) { - return (*env)->GetStaticFieldID(env, type_class, "REQUEST", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "REQUEST", type_sig); } else if (type == BSG_CRUMB_STATE) { - return (*env)->GetStaticFieldID(env, type_class, "STATE", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "STATE", type_sig); } else { // MANUAL is the default type - return (*env)->GetStaticFieldID(env, type_class, "MANUAL", type_sig); + return bsg_safe_get_static_field_id(env, type_class, "MANUAL", type_sig); } } void bugsnag_leave_breadcrumb_env(JNIEnv *env, char *message, bugsnag_breadcrumb_type type) { + jclass interface_class = NULL; + jmethodID leave_breadcrumb_method = NULL; + jclass type_class = NULL; + jfieldID crumb_type = NULL; + jobject jtype = NULL; + jbyteArray jmessage = NULL; + // lookup com/bugsnag/android/NativeInterface - jclass interface_class = + interface_class = bsg_safe_find_class(env, "com/bugsnag/android/NativeInterface"); if (interface_class == NULL) { - return; + goto exit; } // lookup NativeInterface.leaveBreadcrumb() - jmethodID leave_breadcrumb_method = bsg_safe_get_static_method_id( + leave_breadcrumb_method = bsg_safe_get_static_method_id( env, interface_class, "leaveBreadcrumb", "([BLcom/bugsnag/android/BreadcrumbType;)V"); - if (interface_class == NULL) { - return; + if (leave_breadcrumb_method == NULL) { + goto exit; } // lookup com/bugsnag/android/BreadcrumbType - jclass type_class = - bsg_safe_find_class(env, "com/bugsnag/android/BreadcrumbType"); - if (interface_class == NULL) { - return; + type_class = bsg_safe_find_class(env, "com/bugsnag/android/BreadcrumbType"); + if (type_class == NULL) { + goto exit; } - jobject jtype = (*env)->GetStaticObjectField( - env, type_class, bsg_parse_jcrumb_type(env, type, type_class)); - jbyteArray jmessage = bsg_byte_ary_from_string(env, message); - (*env)->CallStaticVoidMethod(env, interface_class, leave_breadcrumb_method, - jmessage, jtype); + // get breadcrumb type fieldID + crumb_type = bsg_parse_jcrumb_type(env, type, type_class); + if (crumb_type == NULL) { + goto exit; + } + + // get the breadcrumb type + jtype = bsg_safe_get_static_object_field(env, type_class, crumb_type); + if (jtype == NULL) { + goto exit; + } + jmessage = bsg_byte_ary_from_string(env, message); + bsg_safe_call_static_void_method(env, interface_class, + leave_breadcrumb_method, jmessage, jtype); + + goto exit; + +exit: bsg_release_byte_ary(env, jmessage, message); + (*env)->DeleteLocalRef(env, interface_class); + (*env)->DeleteLocalRef(env, leave_breadcrumb_method); + (*env)->DeleteLocalRef(env, type_class); + (*env)->DeleteLocalRef(env, crumb_type); (*env)->DeleteLocalRef(env, jtype); (*env)->DeleteLocalRef(env, jmessage); - - (*env)->DeleteLocalRef(env, type_class); - (*env)->DeleteLocalRef(env, interface_class); } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c index b6299d81ae..5b7357738d 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c @@ -185,8 +185,8 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath( // call NativeInterface.deliverReport() jstring japi_key = bsg_safe_new_string_utf(env, event->api_key); if (japi_key != NULL) { - (*env)->CallStaticVoidMethod(env, interface_class, jdeliver_method, - jstage, jpayload, japi_key); + bsg_safe_call_static_void_method(env, interface_class, jdeliver_method, + jstage, jpayload, japi_key); } (*env)->DeleteLocalRef(env, japi_key); } else { @@ -200,18 +200,18 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath( (*env)->ReleaseStringUTFChars(env, _report_path, event_path); goto exit; - exit: - pthread_mutex_unlock(&bsg_native_delivery_mutex); - if (jpayload != NULL) { - (*env)->ReleaseByteArrayElements(env, jpayload, (jbyte *)payload, - 0); // <-- frees payload - } - if (jstage != NULL) { - (*env)->ReleaseByteArrayElements( - env, jstage, (jbyte *)event->app.release_stage, JNI_COMMIT); - } - (*env)->DeleteLocalRef(env, jpayload); - (*env)->DeleteLocalRef(env, jstage); +exit: + pthread_mutex_unlock(&bsg_native_delivery_mutex); + if (jpayload != NULL) { + (*env)->ReleaseByteArrayElements(env, jpayload, (jbyte *)payload, + 0); // <-- frees payload + } + if (jstage != NULL) { + (*env)->ReleaseByteArrayElements( + env, jstage, (jbyte *)event->app.release_stage, JNI_COMMIT); + } + (*env)->DeleteLocalRef(env, jpayload); + (*env)->DeleteLocalRef(env, jstage); } JNIEXPORT void JNICALL diff --git a/bugsnag-plugin-android-ndk/src/main/jni/metadata.c b/bugsnag-plugin-android-ndk/src/main/jni/metadata.c index c2ccd0b7b4..015375531f 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/metadata.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/metadata.c @@ -250,7 +250,7 @@ jobject bsg_get_map_value_obj(JNIEnv *env, bsg_jni_cache *jni_cache, } jobject obj = - (*env)->CallObjectMethod(env, map, jni_cache->hash_map_get, key); + bsg_safe_call_object_method(env, map, jni_cache->hash_map_get, key); (*env)->DeleteLocalRef(env, key); return obj; } @@ -321,7 +321,7 @@ int bsg_populate_cpu_abi_from_map(JNIEnv *env, bsg_jni_cache *jni_cache, } jobjectArray _value = - (*env)->CallObjectMethod(env, map, jni_cache->hash_map_get, key); + bsg_safe_call_object_method(env, map, jni_cache->hash_map_get, key); if (_value != NULL) { int count = (*env)->GetArrayLength(env, _value); @@ -346,30 +346,40 @@ int bsg_populate_cpu_abi_from_map(JNIEnv *env, bsg_jni_cache *jni_cache, void bsg_populate_crumb_metadata(JNIEnv *env, bugsnag_breadcrumb *crumb, jobject metadata) { + bsg_jni_cache *jni_cache = NULL; + jobject keyset = NULL; + jobject keylist = NULL; + if (metadata == NULL) { - return; + goto exit; } - bsg_jni_cache *jni_cache = bsg_populate_jni_cache(env); + jni_cache = bsg_populate_jni_cache(env); if (jni_cache == NULL) { - return; + goto exit; } // get size of metadata map jint map_size = bsg_safe_call_int_method(env, metadata, jni_cache->map_size); if (map_size == -1) { - return; + goto exit; } - jobject keyset = - (*env)->CallObjectMethod(env, metadata, jni_cache->map_key_set); - jobject keylist = (*env)->NewObject( - env, jni_cache->arraylist, jni_cache->arraylist_init_with_obj, keyset); + // create a list of metadata keys + keyset = bsg_safe_call_object_method(env, metadata, jni_cache->map_key_set); + if (keyset == NULL) { + goto exit; + } + keylist = bsg_safe_new_object(env, jni_cache->arraylist, + jni_cache->arraylist_init_with_obj, keyset); + if (keylist == NULL) { + goto exit; + } for (int i = 0; i < map_size; i++) { - jstring _key = (*env)->CallObjectMethod(env, keylist, - jni_cache->arraylist_get, (jint)i); + jstring _key = bsg_safe_call_object_method( + env, keylist, jni_cache->arraylist_get, (jint)i); jobject _value = - (*env)->CallObjectMethod(env, metadata, jni_cache->map_get, _key); + bsg_safe_call_object_method(env, metadata, jni_cache->map_get, _key); if (_key == NULL || _value == NULL) { (*env)->DeleteLocalRef(env, _key); @@ -381,6 +391,9 @@ void bsg_populate_crumb_metadata(JNIEnv *env, bugsnag_breadcrumb *crumb, (*env)->ReleaseStringUTFChars(env, _key, key); } } + goto exit; + +exit: free(jni_cache); (*env)->DeleteLocalRef(env, keyset); (*env)->DeleteLocalRef(env, keylist); @@ -402,8 +415,11 @@ char *bsg_binary_arch() { void bsg_populate_app_data(JNIEnv *env, bsg_jni_cache *jni_cache, bugsnag_event *event) { - jobject data = (*env)->CallStaticObjectMethod( + jobject data = bsg_safe_call_static_object_method( env, jni_cache->native_interface, jni_cache->get_app_data); + if (data == NULL) { + return; + } bsg_strncpy_safe(event->app.binary_arch, bsg_binary_arch(), sizeof(event->app.binary_arch)); @@ -479,8 +495,11 @@ void populate_device_metadata(JNIEnv *env, bsg_jni_cache *jni_cache, void bsg_populate_device_data(JNIEnv *env, bsg_jni_cache *jni_cache, bugsnag_event *event) { - jobject data = (*env)->CallStaticObjectMethod( + jobject data = bsg_safe_call_static_object_method( env, jni_cache->native_interface, jni_cache->get_device_data); + if (data == NULL) { + return; + } bsg_populate_cpu_abi_from_map(env, jni_cache, data, &event->device); @@ -528,8 +547,11 @@ void bsg_populate_device_data(JNIEnv *env, bsg_jni_cache *jni_cache, void bsg_populate_user_data(JNIEnv *env, bsg_jni_cache *jni_cache, bugsnag_event *event) { - jobject data = (*env)->CallStaticObjectMethod( + jobject data = bsg_safe_call_static_object_method( env, jni_cache->native_interface, jni_cache->get_user_data); + if (data == NULL) { + return; + } bsg_copy_map_value_string(env, jni_cache, data, "id", event->user.id, sizeof(event->user.id)); bsg_copy_map_value_string(env, jni_cache, data, "name", event->user.name, @@ -541,7 +563,7 @@ void bsg_populate_user_data(JNIEnv *env, bsg_jni_cache *jni_cache, void bsg_populate_context(JNIEnv *env, bsg_jni_cache *jni_cache, bugsnag_event *event) { - jstring _context = (*env)->CallStaticObjectMethod( + jstring _context = bsg_safe_call_static_object_method( env, jni_cache->native_interface, jni_cache->get_context); if (_context != NULL) { const char *value = (*env)->GetStringUTFChars(env, (jstring)_context, 0); @@ -584,61 +606,115 @@ void bsg_populate_metadata_value(JNIEnv *env, bugsnag_metadata *dst, } } +void bsg_populate_metadata_obj(JNIEnv *env, bugsnag_metadata *dst, + bsg_jni_cache *jni_cache, char *section, + jobject section_keylist, int index) { + jstring section_key = bsg_safe_call_object_method( + env, section_keylist, jni_cache->arraylist_get, (jint)index); + if (section_key == NULL) { + return; + } + char *name = (char *)(*env)->GetStringUTFChars(env, section_key, 0); + jobject _value = bsg_safe_call_object_method(env, section, jni_cache->map_get, + section_key); + bsg_populate_metadata_value(env, dst, jni_cache, section, name, _value); + (*env)->ReleaseStringUTFChars(env, section_key, name); + (*env)->DeleteLocalRef(env, _value); +} + +void bsg_populate_metadata_section(JNIEnv *env, bugsnag_metadata *dst, + jobject metadata, bsg_jni_cache *jni_cache, + jobject keylist, int i) { + jstring _key = NULL; + char *section = NULL; + jobject _section = NULL; + jobject section_keyset = NULL; + jobject section_keylist = NULL; + + _key = bsg_safe_call_object_method(env, keylist, jni_cache->arraylist_get, + (jint)i); + if (_key == NULL) { + goto exit; + } + section = (char *)(*env)->GetStringUTFChars(env, _key, 0); + _section = + bsg_safe_call_object_method(env, metadata, jni_cache->map_get, _key); + if (_section == NULL) { + goto exit; + } + jint section_size = + bsg_safe_call_int_method(env, _section, jni_cache->map_size); + if (section_size == -1) { + goto exit; + } + section_keyset = + bsg_safe_call_object_method(env, _section, jni_cache->map_key_set); + if (section_keyset == NULL) { + goto exit; + } + + section_keylist = + bsg_safe_new_object(env, jni_cache->arraylist, + jni_cache->arraylist_init_with_obj, section_keyset); + if (section_keylist == NULL) { + goto exit; + } + for (int j = 0; j < section_size; j++) { + bsg_populate_metadata_obj(env, dst, jni_cache, section, section_keylist, j); + } + goto exit; + +exit: + (*env)->ReleaseStringUTFChars(env, _key, section); + (*env)->DeleteLocalRef(env, section_keyset); + (*env)->DeleteLocalRef(env, section_keylist); + (*env)->DeleteLocalRef(env, _section); +} + void bsg_populate_metadata(JNIEnv *env, bugsnag_metadata *dst, jobject metadata) { + jobject keyset = NULL; + jobject keylist = NULL; bsg_jni_cache *jni_cache = bsg_populate_jni_cache(env); + if (jni_cache == NULL) { - return; + goto exit; } if (metadata == NULL) { - metadata = (*env)->CallStaticObjectMethod(env, jni_cache->native_interface, - jni_cache->get_metadata); + metadata = bsg_safe_call_static_object_method( + env, jni_cache->native_interface, jni_cache->get_metadata); } if (metadata != NULL) { int size = bsg_safe_call_int_method(env, metadata, jni_cache->map_size); if (size == -1) { - return; + goto exit; + } + + // create a list of metadata keys + keyset = bsg_safe_call_static_object_method(env, metadata, + jni_cache->map_key_set); + if (keyset == NULL) { + goto exit; + } + keylist = bsg_safe_new_object(env, jni_cache->arraylist, + jni_cache->arraylist_init_with_obj, keyset); + if (keylist == NULL) { + goto exit; } - jobject keyset = - (*env)->CallObjectMethod(env, metadata, jni_cache->map_key_set); - jobject keylist = (*env)->NewObject( - env, jni_cache->arraylist, jni_cache->arraylist_init_with_obj, keyset); for (int i = 0; i < size; i++) { - jstring _key = (*env)->CallObjectMethod( - env, keylist, jni_cache->arraylist_get, (jint)i); - char *section = (char *)(*env)->GetStringUTFChars(env, _key, 0); - jobject _section = - (*env)->CallObjectMethod(env, metadata, jni_cache->map_get, _key); - jint section_size = - bsg_safe_call_int_method(env, _section, jni_cache->map_size); - if (section_size == -1) { - break; - } - jobject section_keyset = - (*env)->CallObjectMethod(env, _section, jni_cache->map_key_set); - jobject section_keylist = - (*env)->NewObject(env, jni_cache->arraylist, - jni_cache->arraylist_init_with_obj, section_keyset); - for (int j = 0; j < section_size; j++) { - jstring section_key = (*env)->CallObjectMethod( - env, section_keylist, jni_cache->arraylist_get, (jint)j); - char *name = (char *)(*env)->GetStringUTFChars(env, section_key, 0); - jobject _value = (*env)->CallObjectMethod( - env, section, jni_cache->map_get, section_key); - bsg_populate_metadata_value(env, dst, jni_cache, section, name, _value); - (*env)->ReleaseStringUTFChars(env, section_key, name); - (*env)->DeleteLocalRef(env, _value); - } - (*env)->ReleaseStringUTFChars(env, _key, section); - (*env)->DeleteLocalRef(env, section_keyset); - (*env)->DeleteLocalRef(env, section_keylist); - (*env)->DeleteLocalRef(env, _section); + bsg_populate_metadata_section(env, dst, metadata, jni_cache, keylist, i); } - (*env)->DeleteLocalRef(env, keyset); - (*env)->DeleteLocalRef(env, keylist); } else { dst->value_count = 0; } - free(jni_cache); + goto exit; + +// cleanup +exit: + if (jni_cache != NULL) { + free(jni_cache); + } + (*env)->DeleteLocalRef(env, keyset); + (*env)->DeleteLocalRef(env, keylist); } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/safejni.c b/bugsnag-plugin-android-ndk/src/main/jni/safejni.c index 2c777282d1..f3a62a7ee5 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/safejni.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/safejni.c @@ -11,6 +11,9 @@ bool bsg_check_and_clear_exc(JNIEnv *env) { } jclass bsg_safe_find_class(JNIEnv *env, const char *clz_name) { + if (clz_name == NULL) { + return NULL; + } jclass clz = (*env)->FindClass(env, clz_name); bsg_check_and_clear_exc(env); return clz; @@ -18,6 +21,9 @@ jclass bsg_safe_find_class(JNIEnv *env, const char *clz_name) { jmethodID bsg_safe_get_method_id(JNIEnv *env, jclass clz, const char *name, const char *sig) { + if (clz == NULL || name == NULL || sig == NULL) { + return NULL; + } jmethodID methodId = (*env)->GetMethodID(env, clz, name, sig); bsg_check_and_clear_exc(env); return methodId; @@ -25,12 +31,18 @@ jmethodID bsg_safe_get_method_id(JNIEnv *env, jclass clz, const char *name, jmethodID bsg_safe_get_static_method_id(JNIEnv *env, jclass clz, const char *name, const char *sig) { + if (clz == NULL || name == NULL || sig == NULL) { + return NULL; + } jmethodID methodId = (*env)->GetStaticMethodID(env, clz, name, sig); bsg_check_and_clear_exc(env); return methodId; } jstring bsg_safe_new_string_utf(JNIEnv *env, const char *str) { + if (str == NULL) { + return NULL; + } jstring jstr = (*env)->NewStringUTF(env, str); bsg_check_and_clear_exc(env); return jstr; @@ -38,6 +50,9 @@ jstring bsg_safe_new_string_utf(JNIEnv *env, const char *str) { jboolean bsg_safe_call_boolean_method(JNIEnv *env, jobject _value, jmethodID method) { + if (_value == NULL) { + return false; + } jboolean value = (*env)->CallBooleanMethod(env, _value, method); if (bsg_check_and_clear_exc(env)) { return false; // default to false @@ -46,6 +61,9 @@ jboolean bsg_safe_call_boolean_method(JNIEnv *env, jobject _value, } jint bsg_safe_call_int_method(JNIEnv *env, jobject _value, jmethodID method) { + if (_value == NULL) { + return -1; + } jint value = (*env)->CallIntMethod(env, _value, method); if (bsg_check_and_clear_exc(env)) { return -1; // default to -1 @@ -55,6 +73,9 @@ jint bsg_safe_call_int_method(JNIEnv *env, jobject _value, jmethodID method) { jfloat bsg_safe_call_float_method(JNIEnv *env, jobject _value, jmethodID method) { + if (_value == NULL) { + return -1; + } jfloat value = (*env)->CallFloatMethod(env, _value, method); if (bsg_check_and_clear_exc(env)) { return -1; // default to -1 @@ -64,6 +85,9 @@ jfloat bsg_safe_call_float_method(JNIEnv *env, jobject _value, jdouble bsg_safe_call_double_method(JNIEnv *env, jobject _value, jmethodID method) { + if (_value == NULL) { + return -1; + } jdouble value = (*env)->CallDoubleMethod(env, _value, method); if (bsg_check_and_clear_exc(env)) { return -1; // default to -1 @@ -72,6 +96,9 @@ jdouble bsg_safe_call_double_method(JNIEnv *env, jobject _value, } jobjectArray bsg_safe_new_object_array(JNIEnv *env, jsize size, jclass clz) { + if (clz == NULL) { + return NULL; + } jobjectArray trace = (*env)->NewObjectArray(env, size, clz, NULL); bsg_check_and_clear_exc(env); return trace; @@ -79,6 +106,9 @@ jobjectArray bsg_safe_new_object_array(JNIEnv *env, jsize size, jclass clz) { jobject bsg_safe_get_object_array_element(JNIEnv *env, jobjectArray array, jsize size) { + if (array == NULL) { + return NULL; + } jobject obj = (*env)->GetObjectArrayElement(env, array, size); bsg_check_and_clear_exc(env); return obj; @@ -86,10 +116,83 @@ jobject bsg_safe_get_object_array_element(JNIEnv *env, jobjectArray array, void bsg_safe_set_object_array_element(JNIEnv *env, jobjectArray array, jsize size, jobject object) { + if (array == NULL) { + return; + } (*env)->SetObjectArrayElement(env, array, size, object); bsg_check_and_clear_exc(env); } +jfieldID bsg_safe_get_static_field_id(JNIEnv *env, jclass clz, const char *name, + const char *sig) { + if (clz == NULL || name == NULL || sig == NULL) { + return NULL; + } + jfieldID field_id = (*env)->GetStaticFieldID(env, clz, name, sig); + bsg_check_and_clear_exc(env); + return field_id; +} + +jobject bsg_safe_get_static_object_field(JNIEnv *env, jclass clz, + jfieldID field) { + if (clz == NULL) { + return NULL; + } + jobject obj = (*env)->GetStaticObjectField(env, clz, field); + bsg_check_and_clear_exc(env); + return obj; +} + +jobject bsg_safe_new_object(JNIEnv *env, jclass clz, jmethodID method, ...) { + if (clz == NULL) { + return NULL; + } + va_list args; + va_start(args, method); + jobject obj = (*env)->NewObjectV(env, clz, method, args); + va_end(args); + bsg_check_and_clear_exc(env); + return obj; +} + +jobject bsg_safe_call_object_method(JNIEnv *env, jobject _value, + jmethodID method, ...) { + if (_value == NULL) { + return NULL; + } + va_list args; + va_start(args, method); + jobject value = (*env)->CallObjectMethodV(env, _value, method, args); + va_end(args); + bsg_check_and_clear_exc(env); + return value; +} + +void bsg_safe_call_static_void_method(JNIEnv *env, jclass clz, jmethodID method, + ...) { + if (clz == NULL) { + return; + } + va_list args; + va_start(args, method); + (*env)->CallStaticVoidMethodV(env, clz, method, args); + va_end(args); + bsg_check_and_clear_exc(env); +} + +jobject bsg_safe_call_static_object_method(JNIEnv *env, jclass clz, + jmethodID method, ...) { + if (clz == NULL) { + return NULL; + } + va_list args; + va_start(args, method); + jobject obj = (*env)->CallStaticObjectMethodV(env, clz, method, args); + va_end(args); + bsg_check_and_clear_exc(env); + return obj; +} + jbyteArray bsg_byte_ary_from_string(JNIEnv *env, const char *text) { if (text == NULL) { return NULL; diff --git a/bugsnag-plugin-android-ndk/src/main/jni/safejni.h b/bugsnag-plugin-android-ndk/src/main/jni/safejni.h index 5ac9f27432..edfea7f958 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/safejni.h +++ b/bugsnag-plugin-android-ndk/src/main/jni/safejni.h @@ -3,13 +3,20 @@ #include +/** + * This provides safe JNI calls by wrapping functions and calling + * ExceptionClear(). This approach prevents crashes and undefined behaviour, + * providing the caller checks the return value of each invocation. + * + * For an overview of the methods decorated here, please see + * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html + */ + /** * A safe wrapper for the JNI's FindClass. This method checks if an exception is * pending and if so clears it so that execution can continue. * The caller is responsible for handling the invalid return value of NULL. * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#findclass * @return the class, or NULL if the class could not be found. */ jclass bsg_safe_find_class(JNIEnv *env, const char *clz_name); @@ -19,8 +26,6 @@ jclass bsg_safe_find_class(JNIEnv *env, const char *clz_name); * is pending and if so clears it so that execution can continue. * The caller is responsible for handling the invalid return value of NULL. * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#getmethodid * @return the method ID, or NULL if the method could not be found. */ jmethodID bsg_safe_get_method_id(JNIEnv *env, jclass clz, const char *name, @@ -31,8 +36,6 @@ jmethodID bsg_safe_get_method_id(JNIEnv *env, jclass clz, const char *name, * exception is pending and if so clears it so that execution can continue. * The caller is responsible for handling the invalid return value of NULL. * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#getstaticmethodid * @return the method ID, or NULL if the method could not be found. */ jmethodID bsg_safe_get_static_method_id(JNIEnv *env, jclass clz, @@ -42,8 +45,6 @@ jmethodID bsg_safe_get_static_method_id(JNIEnv *env, jclass clz, * exception is pending and if so clears it so that execution can continue. * The caller is responsible for handling the invalid return value of NULL. * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#newstringutf * @return the java string or NULL if it could not be created */ jstring bsg_safe_new_string_utf(JNIEnv *env, const char *str); @@ -52,8 +53,6 @@ jstring bsg_safe_new_string_utf(JNIEnv *env, const char *str); * A safe wrapper for the JNI's CallBooleanMethod. This method checks if an * exception is pending and if so clears it so that execution can continue. * If an exception was thrown this method returns false. - * - * See https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html */ jboolean bsg_safe_call_boolean_method(JNIEnv *env, jobject _value, jmethodID method); @@ -62,8 +61,6 @@ jboolean bsg_safe_call_boolean_method(JNIEnv *env, jobject _value, * A safe wrapper for the JNI's CallIntMethod. This method checks if an * exception is pending and if so clears it so that execution can continue. * If an exception was thrown this method returns -1. - * - * See https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html */ jint bsg_safe_call_int_method(JNIEnv *env, jobject _value, jmethodID method); @@ -71,8 +68,6 @@ jint bsg_safe_call_int_method(JNIEnv *env, jobject _value, jmethodID method); * A safe wrapper for the JNI's CallFloatMethod. This method checks if an * exception is pending and if so clears it so that execution can continue. * If an exception was thrown this method returns -1. - * - * See https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html */ jfloat bsg_safe_call_float_method(JNIEnv *env, jobject _value, jmethodID method); @@ -81,8 +76,6 @@ jfloat bsg_safe_call_float_method(JNIEnv *env, jobject _value, * A safe wrapper for the JNI's CallDoubleMethod. This method checks if an * exception is pending and if so clears it so that execution can continue. * If an exception was thrown this method returns -1. - * - * See https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html */ jdouble bsg_safe_call_double_method(JNIEnv *env, jobject _value, jmethodID method); @@ -91,9 +84,6 @@ jdouble bsg_safe_call_double_method(JNIEnv *env, jobject _value, * A safe wrapper for the JNI's NewObjectArray. This method checks if an * exception is pending and if so clears it so that execution can continue. * The caller is responsible for handling the invalid return value of NULL. - * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#newobjectarray */ jobjectArray bsg_safe_new_object_array(JNIEnv *env, jsize size, jclass clz); @@ -101,9 +91,6 @@ jobjectArray bsg_safe_new_object_array(JNIEnv *env, jsize size, jclass clz); * A safe wrapper for the JNI's GetObjectArrayElement. This method checks if an * exception is pending and if so clears it so that execution can continue. * The caller is responsible for handling the invalid return value of NULL. - * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#getobjectarrayelement */ jobject bsg_safe_get_object_array_element(JNIEnv *env, jobjectArray array, jsize size); @@ -111,13 +98,55 @@ jobject bsg_safe_get_object_array_element(JNIEnv *env, jobjectArray array, /** * A safe wrapper for the JNI's SetObjectArrayElement. This method checks if an * exception is pending and if so clears it so that execution can continue. - * - * See - * https://docs.oracle.com/en/java/javase/11/docs/specs/jni/functions.html#setobjectarrayelement */ void bsg_safe_set_object_array_element(JNIEnv *env, jobjectArray array, jsize size, jobject object); +/** + * A safe wrapper for the JNI's GetStaticFieldId. This method checks if an + * exception is pending and if so clears it so that execution can continue. + */ +jfieldID bsg_safe_get_static_field_id(JNIEnv *env, jclass clz, const char *name, + const char *sig); + +/** + * A safe wrapper for the JNI's GetStaticObjectField. This method checks if an + * exception is pending and if so clears it so that execution can continue. + * The caller is responsible for handling the invalid return value of NULL. + */ +jobject bsg_safe_get_static_object_field(JNIEnv *env, jclass clz, + jfieldID field); + +/** + * A safe wrapper for the JNI's NewObject. This method checks if an + * exception is pending and if so clears it so that execution can continue. + * The caller is responsible for handling the invalid return value of NULL. + */ +jobject bsg_safe_new_object(JNIEnv *env, jclass clz, jmethodID method, ...); + +/** + * A safe wrapper for the JNI's CallObjectMethod. This method checks if an + * exception is pending and if so clears it so that execution can continue. + * The caller is responsible for handling the invalid return value of NULL. + */ +jobject bsg_safe_call_object_method(JNIEnv *env, jobject _value, + jmethodID method, ...); + +/** + * A safe wrapper for the JNI's CallStaticVoidMethod. This method checks if an + * exception is pending and if so clears it so that execution can continue. + */ +void bsg_safe_call_static_void_method(JNIEnv *env, jclass clz, jmethodID method, + ...); + +/** + * A safe wrapper for the JNI's CallStaticObjectMethod. This method checks if an + * exception is pending and if so clears it so that execution can continue. + * The caller is responsible for handling the invalid return value of NULL. + */ +jobject bsg_safe_call_static_object_method(JNIEnv *env, jclass clz, + jmethodID method, ...); + /** * Constructs a byte array from a string. */