Browse Source

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.
Max Bruckner 8 years ago
parent
commit
954d61e5e7
2 changed files with 25 additions and 1 deletions
  1. 8 0
      cJSON.c
  2. 17 1
      tests/misc_tests.c

+ 8 - 0
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
     {

+ 17 - 1
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();
 }