Fix potential segfaults when adding breadcrumb with NDK

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## TBD
+
+### Bug fixes
+* Fix potential segfaults when adding breadcrumb with NDK
+  [#546](https://github.com/bugsnag/bugsnag-android/pull/546)
+
 ## 4.17.1 (2019-07-24)
 
 ### Bug fixes

bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BreadcrumbMutabilityTest.kt

@@ -0,0 +1,26 @@
+package com.bugsnag.android
+
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNotSame
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class BreadcrumbMutabilityTest {
+
+    @Test
+    fun breadcrumbCopiesMap() {
+        val data = mutableMapOf<String, String>()
+        val breadcrumb = Breadcrumb("foo", BreadcrumbType.MANUAL, data)
+        data["a"] = "bar"
+        assertTrue(breadcrumb.metadata.isEmpty())
+        assertNotSame(data, breadcrumb.metadata)
+    }
+
+    @Test
+    fun breadcrumbProtectsMetaData() {
+        val data = mutableMapOf<String, String>()
+        val breadcrumb = Breadcrumb("foo", BreadcrumbType.MANUAL, data)
+        breadcrumb.metadata["a"] = "bar"
+        assertFalse(breadcrumb.metadata.isEmpty())
+    }
+}

bugsnag-android-core/src/main/java/com/bugsnag/android/Breadcrumb.java

@@ -7,6 +7,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -40,10 +41,7 @@ public final class Breadcrumb implements JsonStream.Streamable {
     Breadcrumb(@NonNull String name,
                @NonNull BreadcrumbType type,
                @NonNull Map<String, String> metadata) {
-        this.timestamp = DateUtils.toIso8601(new Date());
-        this.type = type;
-        this.metadata = metadata;
-        this.name = name;
+        this(name, type, new Date(), metadata);
     }
 
     Breadcrumb(@NonNull String name,
@@ -52,8 +50,8 @@ public final class Breadcrumb implements JsonStream.Streamable {
                @NonNull Map<String, String> metadata) {
         this.timestamp = DateUtils.toIso8601(captureDate);
         this.type = type;
-        this.metadata = metadata;
         this.name = name;
+        this.metadata = new HashMap<>(metadata);
     }
 
     @NonNull

bugsnag-plugin-android-ndk/src/main/jni/metadata.c

@@ -178,18 +178,24 @@ 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) {
+  if (metadata == NULL) {
+    return;
+  }
+
   bsg_jni_cache *jni_cache = bsg_populate_jni_cache(env);
-  int size = (int)(*env)->CallIntMethod(env, metadata, jni_cache->map_size);
+  int map_size = (int)(*env)->CallIntMethod(env, metadata, jni_cache->map_size);
   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 < sizeof(crumb->metadata); i++) {
+  size_t metadata_size = sizeof(crumb->metadata) / sizeof(bsg_char_metadata_pair);
+
+  for (int i = 0; i < map_size && i < metadata_size; i++) {
     jstring _key = (*env)->CallObjectMethod(env, keylist,
                                             jni_cache->arraylist_get, (jint)i);
     jstring _value =
         (*env)->CallObjectMethod(env, metadata, jni_cache->map_get, _key);
-    if (_value == NULL) {
+    if (_key == NULL || _value == NULL) {
       (*env)->DeleteLocalRef(env, _key);
       (*env)->DeleteLocalRef(env, _value);
     } else {