cJSON icon indicating copy to clipboard operation
cJSON copied to clipboard

Double Free Vulnerability Discovered in cJSON_Delete Function Through Fuzzing

Open wyywang opened this issue 4 months ago • 6 comments

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
}

wyywang avatar Feb 29 '24 05:02 wyywang

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?

Up-wind avatar Apr 08 '24 11:04 Up-wind

@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.

wyywang avatar Apr 08 '24 11:04 wyywang

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

PeterAlfredLee avatar Apr 26 '24 08:04 PeterAlfredLee

A pr is always welcome. :)

Alanscut avatar Apr 26 '24 08:04 Alanscut

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.

wyywang avatar May 09 '24 09:05 wyywang

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

PeterAlfredLee avatar May 10 '24 10:05 PeterAlfredLee