From 22a7d04fa004462e0dca35c3cc7809bea38e65f9 Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Fri, 2 Mar 2018 19:49:55 +0100 Subject: [PATCH 1/2] add_item_to_object: Fix use-after-free when string is aliased If the `string` property of the item that is added is an alias to the `string` parameter of `add_item_to_object`, and `constant` is false, `cJSON_strdup` would access the string after it has been freed. Thanks @hhallen for reporting this in #248. --- cJSON.c | 37 +++++++++++++++++++++---------------- tests/misc_tests.c | 20 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/cJSON.c b/cJSON.c index 3dcde3d..cf48ac9 100644 --- a/cJSON.c +++ b/cJSON.c @@ -1895,32 +1895,37 @@ static void* cast_away_const(const void* string) static cJSON_bool add_item_to_object(cJSON * const object, const char * const string, cJSON * const item, const internal_hooks * const hooks, const cJSON_bool constant_key) { + char *new_key = NULL; + int new_type = cJSON_Invalid; + if ((object == NULL) || (string == NULL) || (item == NULL)) { return false; } + if (constant_key) + { + new_key = (char*)cast_away_const(string); + new_type = item->type | cJSON_StringIsConst; + } + else + { + new_key = (char*)cJSON_strdup((const unsigned char*)string, hooks); + if (new_key == NULL) + { + return false; + } + + new_type = item->type & ~cJSON_StringIsConst; + } + if (!(item->type & cJSON_StringIsConst) && (item->string != NULL)) { hooks->deallocate(item->string); } - if (constant_key) - { - item->string = (char*)cast_away_const(string); - item->type |= cJSON_StringIsConst; - } - else - { - char *key = (char*)cJSON_strdup((const unsigned char*)string, hooks); - if (key == NULL) - { - return false; - } - - item->string = key; - item->type &= ~cJSON_StringIsConst; - } + item->string = new_key; + item->type = new_type; return add_item_to_array(object, item); } diff --git a/tests/misc_tests.c b/tests/misc_tests.c index a900e8f..a0b4f7e 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -508,6 +508,25 @@ static void cjson_create_array_reference_should_create_an_array_reference(void) cJSON_Delete(number_reference); } +static void cjson_add_item_to_object_should_not_use_after_free_when_string_is_aliased(void) +{ + cJSON *object = cJSON_CreateObject(); + cJSON *number = cJSON_CreateNumber(42); + char *name = (char*)cJSON_strdup((const unsigned char*)"number", &global_hooks); + + TEST_ASSERT_NOT_NULL(object); + TEST_ASSERT_NOT_NULL(number); + TEST_ASSERT_NOT_NULL(name); + + number->string = name; + + /* The following should not have a use after free + * that would show up in valgrind or with AddressSanitizer */ + cJSON_AddItemToObject(object, number->string, number); + + cJSON_Delete(object); +} + int main(void) { UNITY_BEGIN(); @@ -530,6 +549,7 @@ int main(void) RUN_TEST(cjson_create_string_reference_should_create_a_string_reference); RUN_TEST(cjson_create_object_reference_should_create_an_object_reference); RUN_TEST(cjson_create_array_reference_should_create_an_array_reference); + RUN_TEST(cjson_add_item_to_object_should_not_use_after_free_when_string_is_aliased); return UNITY_END(); } From 5da9edc8b145d4f335267a23076c63a90a78b672 Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Fri, 2 Mar 2018 19:57:36 +0100 Subject: [PATCH 2/2] Release version 1.7.4 --- CHANGELOG.md | 8 +++++++- CMakeLists.txt | 2 +- Makefile | 2 +- cJSON.c | 2 +- cJSON.h | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001269b..b40c895 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ -1.7.2 +1.7.4 +===== +Fixes: +------ +* Fix potential use after free if the `string` parameter to `cJSON_AddItemToObject` is an alias of the `string` property of the object that is added (#248). Thanks @hhallen for reporting. + +1.7.3 ===== Fixes: ------ diff --git a/CMakeLists.txt b/CMakeLists.txt index 774280e..41b8b29 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ include(GNUInstallDirs) set(PROJECT_VERSION_MAJOR 1) set(PROJECT_VERSION_MINOR 7) -set(PROJECT_VERSION_PATCH 3) +set(PROJECT_VERSION_PATCH 4) set(CJSON_VERSION_SO 1) set(CJSON_UTILS_VERSION_SO 1) set(PROJECT_VERSION "${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}") diff --git a/Makefile b/Makefile index 1b4552b..01ee754 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ CJSON_TEST_SRC = cJSON.c test.c LDLIBS = -lm -LIBVERSION = 1.7.3 +LIBVERSION = 1.7.4 CJSON_SOVERSION = 1 UTILS_SOVERSION = 1 diff --git a/cJSON.c b/cJSON.c index cf48ac9..b4fc727 100644 --- a/cJSON.c +++ b/cJSON.c @@ -82,7 +82,7 @@ CJSON_PUBLIC(char *) cJSON_GetStringValue(cJSON *item) { } /* This is a safeguard to prevent copy-pasters from using incompatible C and header files */ -#if (CJSON_VERSION_MAJOR != 1) || (CJSON_VERSION_MINOR != 7) || (CJSON_VERSION_PATCH != 3) +#if (CJSON_VERSION_MAJOR != 1) || (CJSON_VERSION_MINOR != 7) || (CJSON_VERSION_PATCH != 4) #error cJSON.h and cJSON.c have different versions. Make sure that both have the same. #endif diff --git a/cJSON.h b/cJSON.h index b6937b3..4dc1cfb 100644 --- a/cJSON.h +++ b/cJSON.h @@ -31,7 +31,7 @@ extern "C" /* project version */ #define CJSON_VERSION_MAJOR 1 #define CJSON_VERSION_MINOR 7 -#define CJSON_VERSION_PATCH 3 +#define CJSON_VERSION_PATCH 4 #include