From 954d61e5e7cb9dc6c480fc28ac1cdceca07dd5bd Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Wed, 12 Jul 2017 22:58:06 +0200 Subject: [PATCH] Fix #189, ensure returns an invalid pointer If realloc returns NULL, ensure didn't abort but returned printbuffer.offset instead. If an attacker can control printbuffer.offset and also make realloc fail at just the right moment, this would make cJSON potentially write at an arbitrary memory address. --- cJSON.c | 8 ++++++++ tests/misc_tests.c | 18 +++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/cJSON.c b/cJSON.c index 314560b..753b215 100644 --- a/cJSON.c +++ b/cJSON.c @@ -377,6 +377,14 @@ static unsigned char* ensure(printbuffer * const p, size_t needed) { /* reallocate with realloc if available */ newbuffer = (unsigned char*)p->hooks.reallocate(p->buffer, newsize); + if (newbuffer == NULL) + { + p->hooks.deallocate(p->buffer); + p->length = 0; + p->buffer = NULL; + + return NULL; + } } else { diff --git a/tests/misc_tests.c b/tests/misc_tests.c index 7d51179..03a6e73 100644 --- a/tests/misc_tests.c +++ b/tests/misc_tests.c @@ -410,6 +410,22 @@ static void cjson_functions_shouldnt_crash_with_null_pointers(void) cJSON_Delete(item); } +static void *failing_realloc(void *pointer, size_t size) +{ + (void)size; + (void)pointer; + return NULL; +} + +static void ensure_should_fail_on_failed_realloc(void) +{ + printbuffer buffer = {NULL, 10, 0, 0, false, false, {&malloc, &free, &failing_realloc}}; + buffer.buffer = (unsigned char*)malloc(100); + TEST_ASSERT_NOT_NULL(buffer.buffer); + + TEST_ASSERT_NULL_MESSAGE(ensure(&buffer, 200), "Ensure didn't fail with failing realloc."); +} + int main(void) { UNITY_BEGIN(); @@ -425,6 +441,6 @@ int main(void) RUN_TEST(cjson_replace_item_via_pointer_should_replace_items); RUN_TEST(cjson_replace_item_in_object_should_preserve_name); RUN_TEST(cjson_functions_shouldnt_crash_with_null_pointers); - + RUN_TEST(ensure_should_fail_on_failed_realloc); return UNITY_END(); }