DaveGamble / cJSON

Ultralightweight JSON parser in ANSI C
MIT License
10.28k stars 3.15k forks source link

Double Free Vulnerability Discovered in cJSON_Delete Function Through Fuzzing #833

Open wyywang opened 4 months ago

wyywang commented 4 months ago

During a recent fuzzing session aimed at identifying memory management vulnerabilities within the cJSON library, a double free issue was uncovered in the cJSON_Delete function. This vulnerability poses a significant risk as it could potentially lead to security flaws like crashes or, in worse scenarios, arbitrary code execution if exploited. The issue arises because the function does not set the freed pointers to NULL, which might lead to their unintended reuse.

CJSON_PUBLIC(void) cJSON_Delete(cJSON *item)
{
    cJSON *next = NULL;
    while (item != NULL)
    {
        next = item->next;
        if (!(item->type & cJSON_IsReference) && (item->child != NULL))
        {
            cJSON_Delete(item->child);
        }
        if (!(item->type & cJSON_IsReference) && (item->valuestring != NULL))
        {
            global_hooks.deallocate(item->valuestring);
        }
        if (!(item->type & cJSON_StringIsConst) && (item->string != NULL))
        {
            global_hooks.deallocate(item->string);
        }
        global_hooks.deallocate(item);
        item = next;
    }
}

To mitigate this issue, it is recommended to set pointers to NULL immediately after they are deallocated. This change will prevent the library from accessing or freeing already freed memory, thus avoiding double free vulnerabilities. Example of suggested change:

if (!(item->type & cJSON_IsReference) && (item->valuestring != NULL)) {
    global_hooks.deallocate(item->valuestring);
    item->valuestring = NULL; // Prevent double free by setting pointer to NULL
}

fuzzcode:

#include "cJSON.h"
#include <stdbool.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
#include <math.h>
typedef uint8_t   u8;   
typedef uint16_t  u16;  
typedef uint32_t  u32;  
typedef uint64_t  u64;
typedef unsigned int usize;
typedef int8_t  i8;
typedef int16_t i16;
typedef int32_t i32;
typedef int64_t i64;
typedef int isize;
typedef float f32;
typedef double f64;
int main() {
    cJSON v0_tmp[] = {{ NULL, NULL, NULL, 1, NULL, 67, -7.939443733432238, NULL,  }, }; // child
    cJSON *v0 = malloc(sizeof v0_tmp);
    memcpy(v0, v0_tmp, sizeof v0_tmp);
    cJSON v1_tmp[] = {{ NULL, NULL, v0, 76, NULL, 35, 62.02300219225285, NULL,  }, }; // parent
    cJSON *v1 = malloc(sizeof v1_tmp);
    memcpy(v1, v1_tmp, sizeof v1_tmp);
    cJSON *v2 = v1; // parent
    cJSON v3_tmp[] = {{ NULL, NULL, NULL, 9, NULL, 32890, 56.250703290040846, NULL,  }, }; // replacement
    cJSON *v3 = malloc(sizeof v3_tmp);
    memcpy(v3, v3_tmp, sizeof v3_tmp);
    cJSON *v4 = v3; // replacement
    i8 v5_tmp[] = {123, 34, 34, 58, 123, 34, 34, 58, 116, 114, 117, 101, 125, 44, 34, 34, 58, 123, 34, 34, 58, 102, 97, 108, 115, 101, 125, 44, 34, 92, 117, 48, 48, -79, 101, 92, 117, 48, 48, 49, 52, 47, 100, -84, 62, 34, 58, 91, 123, 10, 9, 34, -22, 100, -83, 34, 58, 123, 34, 34, 58, 110, 117, 108, 108, 125, 44, 34, 34, 58, 123, 34, 34, 58, 110, 117, 108, 108, 125, 44, 34, 34, 58, 123, 34, 34, 58, 13, 102, 97, 108, 115, 101, 125, 125, 93, 125, -71, -96, -20, -16, 98, -116, 93, 64, 0, 0, 0, 49, 101, 92, 117, 48, 48, 49, 52, 47, 100, -84, 62, 34, 58, 9, 112, 50, -55, -2, -22, 100, -84, 62, 34, 58, 123, 48, 51, 92, 117, -21, 48, 49, 97, 92, 92, 57, 0, }; // value
    i8 *v5 = malloc(sizeof v5_tmp);
    memcpy(v5, v5_tmp, sizeof v5_tmp);
    i8 *v6 = v5; // value
    cJSON *v7 = cJSON_Parse(v6); // item
    if (v7 == NULL) return 0;
    cJSON *v9 = v7; // item
    i8 v10_tmp[] = {35, 0, }; // name
    i8 *v10 = malloc(sizeof v10_tmp);
    memcpy(v10, v10_tmp, sizeof v10_tmp);
    i8 *v11 = v10; // name
    f64 v12 = INFINITY; // number
    cJSON *v13 = cJSON_AddNumberToObject(v9, v11, v12); // $relative
    i8 v14_tmp[] = {82, 0, }; // name
    i8 *v14 = malloc(sizeof v14_tmp);
    memcpy(v14, v14_tmp, sizeof v14_tmp);
    i8 *v15 = v14; // name
    cJSON *v16 = cJSON_AddNullToObject(v9, v15); // $relative
    i32 v17 = 4; // which
    i32 v18 = 4; // which
    cJSON v19_tmp[] = {{ NULL, NULL, NULL, 56, NULL, 1, 7.034879206248807, NULL,  }, }; // next
    cJSON *v19 = malloc(sizeof v19_tmp);
    memcpy(v19, v19_tmp, sizeof v19_tmp);
    cJSON v20_tmp[] = {{ v19, NULL, NULL, 64, NULL, -1, 51.96094189122921, NULL,  }, }; // newitem
    cJSON *v20 = malloc(sizeof v20_tmp);
    memcpy(v20, v20_tmp, sizeof v20_tmp);
    cJSON *v21 = v20; // newitem
    i8 v22_tmp[] = {-97, 0, }; // name
    i8 *v22 = malloc(sizeof v22_tmp);
    memcpy(v22, v22_tmp, sizeof v22_tmp);
    i8 *v23 = v22; // name
    f64 v24 = INFINITY; // number
    cJSON *v25 = cJSON_AddNumberToObject(v21, v23, v24); // $relative
    i8 v26_tmp[] = {4, 0, }; // name
    i8 *v26 = malloc(sizeof v26_tmp);
    memcpy(v26, v26_tmp, sizeof v26_tmp);
    i8 *v27 = v26; // name
    cJSON *v28 = cJSON_AddArrayToObject(v21, v27); // $relative
    i8 v29_tmp[] = {-115, 0, }; // name
    i8 *v29 = malloc(sizeof v29_tmp);
    memcpy(v29, v29_tmp, sizeof v29_tmp);
    i8 *v30 = v29; // name
    cJSON *v31 = cJSON_AddObjectToObject(v21, v30); // $relative
    i32 v32 = cJSON_InsertItemInArray(v9, v17, v21); // $relative
    i8 v33_tmp[] = {-68, 0, }; // name
    i8 *v33 = malloc(sizeof v33_tmp);
    memcpy(v33, v33_tmp, sizeof v33_tmp);
    i8 *v34 = v33; // name
    i32 v35 = 0; // boolean
    cJSON *v36 = cJSON_AddBoolToObject(v9, v34, v35); // $relative
    i32 v37 = cJSON_InsertItemInArray(v9, v18, v21); // $relative
    i32 v38 = cJSON_ReplaceItemViaPointer(v2, v9, v4); // $target
}
Up-wind commented 2 months ago

Hello, I noticed that the style of your fuzzcode is a bit like Hopper. May I ask if you have made any modifications to Hopper to achieve this?

wyywang commented 2 months ago

@Up-wind Thank you for your interest and question. Yes, this work was a collective effort by our team. To achieve the fuzzing results, we indeed made some custom modifications to Hopper, but for various reasons, we are currently unable to open-source this customized version. Fortunately, the double free vulnerability we reported has been identified and fixed. Regarding the assignment of a CVE number, we are also looking forward to it, but that decision rests with the relevant security organizations based on their assessment and processes. We have submitted all the necessary information following the standard procedures and hope to receive a response soon. Thank you again for your attention and understanding.

PeterAlfredLee commented 2 months ago

Looks good. Seems we can also do similar things to item->string.

Alanscut commented 2 months ago

A pr is always welcome. :)

wyywang commented 1 month ago

A pr is always welcome. :)

Thank you for addressing the issue so quickly. To track and document this vulnerability properly, could we initiate the process to obtain a CVE number for the "Double Free Vulnerability in cJSON_Delete Function"? This will help in acknowledging the security implications and ensuring it is recognized and addressed appropriately in various security advisories and databases.

PeterAlfredLee commented 1 month ago

As the recent security policy declares, I think we should discuss in mails.