Merge pull request #962 from junkilee80/fix/object-add-key-leak

Fix object key leak on insertion failure

json_object.c

@@ -753,11 +753,21 @@ int json_object_object_add_ex(struct json_object *jso, const char *const key,
 
 	if (!existing_entry)
 	{
-		const void *const k =
+		char *key_copy = NULL;
-		    (opts & JSON_C_OBJECT_ADD_CONSTANT_KEY) ? (const void *)key : strdup(key);
+		const void *k = key;
-		if (k == NULL)
+		if (!(opts & JSON_C_OBJECT_ADD_CONSTANT_KEY))
+		{
+			key_copy = strdup(key);
+			if (key_copy == NULL)
+				return -1;
+			k = key_copy;
+		}
+		if (lh_table_insert_w_hash(JC_OBJECT(jso)->c_object, k, val, hash, opts) < 0)
+		{
+			free(key_copy);
 			return -1;
-		return lh_table_insert_w_hash(JC_OBJECT(jso)->c_object, k, val, hash, opts);
+		}
+		return 0;
 	}
 	existing_value = (json_object *)lh_entry_v(existing_entry);
 	if (existing_value)

tests/CMakeLists.txt

@@ -28,6 +28,7 @@ set(ALL_TEST_NAMES
     test_int_get
     test_locale
     test_null
+    test_object_add_failure
     test_parse
     test_parse_int64
     test_printbuf

tests/meson.build

@@ -19,6 +19,7 @@ test_cases = [
   ['test2', 'test2.expected'],
   ['test4', 'test4.expected'],
   ['testReplaceExisting', 'testReplaceExisting.expected'],
+  ['test_object_add_failure', 'test_object_add_failure.expected'],
   ['test_cast', 'test_cast.expected'],
   ['test_charcase', 'test_charcase.expected'],
   ['test_compare', 'test_compare.expected'],

tests/test_object_add_failure.c

@@ -0,0 +1,44 @@
+#ifdef NDEBUG
+#undef NDEBUG
+#endif
+#include <assert.h>
+#include <limits.h>
+#include <stdio.h>
+
+#include "json.h"
+
+static void check_insert_failure(const char *key, unsigned int opts)
+{
+	struct json_object *object = json_object_new_object();
+	struct json_object *value = json_object_new_int(7);
+	struct lh_table *table;
+	int saved_count;
+	int saved_size;
+
+	assert(object != NULL);
+	assert(value != NULL);
+	table = json_object_get_object(object);
+	assert(table != NULL);
+
+	/* Force lh_table_insert_w_hash() to reject the new entry. */
+	saved_count = table->count;
+	saved_size = table->size;
+	table->count = INT_MAX;
+	table->size = INT_MAX;
+
+	assert(json_object_object_add_ex(object, key, value,
+	                                 opts | JSON_C_OBJECT_ADD_KEY_IS_NEW) == -1);
+
+	table->count = saved_count;
+	table->size = saved_size;
+	json_object_put(value);
+	json_object_put(object);
+}
+
+int main(void)
+{
+	check_insert_failure("copied-key", 0);
+	check_insert_failure("constant-key", JSON_C_OBJECT_ADD_CONSTANT_KEY);
+	puts("PASS");
+	return 0;
+}

tests/test_object_add_failure.expected

@@ -0,0 +1 @@
+PASS

tests/test_object_add_failure.test

@@ -0,0 +1,15 @@
+#!/bin/sh
+
+# Common definitions
+if test -z "$srcdir"; then
+    srcdir="${0%/*}"
+    test "$srcdir" = "$0" && srcdir=.
+    test -z "$srcdir" && srcdir=.
+fi
+. "$srcdir/test-defs.sh"
+
+filename=$(basename "$0")
+filename="${filename%.*}"
+
+run_output_test $filename
+exit $?