Merge pull request #1918 from bugsnag/PLAT-11084/metrics-safety

Additional null-checks when handling callback names
bugsnag · Oct 25, 2023 · f613f51 · f613f51
2 parents 805001f + b87bba0
commit f613f51
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## TBD
+
+### Bug fixes
+
+* Handle cases in NDK when `GetStringUTFChars` fails for callback names
+  [#1918](https://github.com/bugsnag/bugsnag-android/pull/1918)
+
 ## 5.31.2 (2023-10-11)
 
 ### Bug fixes

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
@@ -955,14 +955,24 @@ Java_com_bugsnag_android_ndk_NativeBridge_notifyAddCallback(JNIEnv *env,
                                                             jobject thiz,
                                                             jstring callback_) {
   const char *callback = bsg_safe_get_string_utf_chars(env, callback_);
+  if (!callback) {
+    return;
+  }
+
   bsg_notify_add_callback(&bsg_global_env->next_event, callback);
+  bsg_safe_release_string_utf_chars(env, callback_, callback);
 }
 
 JNIEXPORT void JNICALL
 Java_com_bugsnag_android_ndk_NativeBridge_notifyRemoveCallback(
     JNIEnv *env, jobject thiz, jstring callback_) {
   const char *callback = bsg_safe_get_string_utf_chars(env, callback_);
+  if (!callback) {
+    return;
+  }
+
   bsg_notify_remove_callback(&bsg_global_env->next_event, callback);
+  bsg_safe_release_string_utf_chars(env, callback_, callback);
 }
 
 JNIEXPORT void JNICALL

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/internal_metrics.c b/bugsnag-plugin-android-ndk/src/main/jni/internal_metrics.c
@@ -130,26 +130,28 @@ static void bsg_modify_callback_count(bugsnag_event *event, const char *api,
                                       int delta) {
   static const int total_callbacks =
       sizeof(event->set_callback_counts) / sizeof(*event->set_callback_counts);
-  if (strlen(api) >= sizeof(event->set_callback_counts[0].name)) {
-    // API name is too big to store.
+  if (!api || strnlen(api, sizeof(event->set_callback_counts[0].name)) >=
+                  sizeof(event->set_callback_counts[0].name)) {
+    // API name is NULL or is too big to store.
     return;
   }
 
   int i = 0;
   for (; i < total_callbacks && event->set_callback_counts[i].name[0] != 0;
        i++) {
-    if (strcmp(event->set_callback_counts[i].name, api) == 0) {
-      event->set_callback_counts[i].count += delta;
-      if (event->set_callback_counts[i].count < 0) {
-        event->set_callback_counts[i].count = 0;
+    set_callback_count *callback_counter = &event->set_callback_counts[i];
+    if (strcmp(callback_counter->name, api) == 0) {
+      callback_counter->count += delta;
+      if (callback_counter->count < 0) {
+        callback_counter->count = 0;
       }
       return;
     }
   }
   if (i < total_callbacks && delta > 0) {
-    strncpy(event->set_callback_counts[i].name, api,
-            sizeof(event->set_callback_counts[i].name));
-    event->set_callback_counts[i].count = delta;
+    set_callback_count *callback_counter = &event->set_callback_counts[i];
+    strncpy(callback_counter->name, api, sizeof(callback_counter->name));
+    callback_counter->count = delta;
   }
 }