DaveGamble / cJSON

Ultralightweight JSON parser in ANSI C
MIT License
10.68k stars 3.21k forks source link

A segmentation fault in cJSON_SetValuestring #839

Closed Up-wind closed 5 months ago

Up-wind commented 6 months ago

Hi,

when fuzzing cJSON library, I found a segmentation fault happened in cJSON_SetValuestring.

If the valuestring passed to cJSON_SetValuestring is NULL, a null pointer dereference will happen in the following statements:

CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring)
{
    ···
    if (object->valuestring == NULL)
    {
        return NULL;
    }
    if (strlen(valuestring) <= strlen(object->valuestring)) // null pointer dereference happens here
    {
        strcpy(object->valuestring, valuestring);
        return object->valuestring;
    }

The PoC is as follows:

    cJSON *item = cJSON_CreateString("apple");
    cJSON_SetValuestring(item, NULL);

The null pointer dereference happens here can potentially cause denial of service (DOS). Maybe we can check it before strlen(), just like object->valuestring did.

Affected Version

commit 87d8f0961a01bf09bef98ff89bae9fdec42181ee (HEAD -> master, tag: v1.7.17, origin/master, origin/HEAD)
Author: Alanscut <wp_scut@163.com>
Date:   Tue Dec 26 10:07:05 2023 +0800
Alanscut commented 5 months ago

Hi @Up-wind Thanks for your job.

IMHO security issues should be discussed in private. Besides this, it will be appreciated to request a CVE after a version containing a fix is released, in which way downstream users can upgrade at the first time when CVE is published.

To achieve this. I will update the GH security pages later.

Up-wind commented 5 months ago

Hi @Alanscut Thank you for teaching me an important lesson.

I apologize for my reckless of discussing a security issue in public and requesting a CVE before a fixed version is released. I hope that this issue will not affect any downstream projects.

I actually quite agree with what you said, but I was just new to this. I’ve seen someone did this before, so I naively thought that it was a proper way to request a CVE.

Sorry again. I will obey the security rules next time.