bug for cJSON_SetValuestring

Du4t commented 11 months ago

Description

If the the object passed in cJSON_SetValuestring dont have valuestring, the object->valuestringwill be null. The null pointer dereference will cause SEGV in function cJSON_SetValuestring cJSON.c:408

Version

commit cb8693b058ba302f4829ec6d03f609ac6f848546 (HEAD -> master, tag: v1.7.16, origin/master, origin/HEAD)
Author: Alan Wang <wp_scut@163.com>
Date:   Wed Jul 5 11:22:19 2023 +0800

Related Code

CJSON_PUBLIC(char*) cJSON_SetValuestring(cJSON *object, const char *valuestring)
{
    char *copy = NULL;
    /* if object's type is not cJSON_String or is cJSON_IsReference, it should not set valuestring */
    if (!(object->type & cJSON_String) || (object->type & cJSON_IsReference))
    {
        return NULL;
    }
    if (strlen(valuestring) <= strlen(object->valuestring)) // <== here
    {
        strcpy(object->valuestring, valuestring);
        return object->valuestring;
    }
    copy = (char*) cJSON_strdup((const unsigned char*)valuestring, &global_hooks);
    if (copy == NULL)
    {
        return NULL;
    }
    if (object->valuestring != NULL)
    {
        cJSON_free(object->valuestring);
    }
    object->valuestring = copy;

    return copy;
}

Impact

Potentially causing DoS

carnil commented 11 months ago

Looks this issue got a CVE assigned, CVE-2023-50472

PeterAlfredLee commented 11 months ago

My POC if I'm understanding this problem correctly:

    cJSON *corruptedItem = cJSON_CreateString("corrupted");

    corruptedItem->valuestring = NULL;
    return_value = cJSON_SetValuestring(corruptedItem, "test");

mmuehlenhoff commented 11 months ago

Why is this considered a security issue? This crosses no security boundary, it only lacks sanity handling for broken use of a function?

carnil commented 11 months ago

Why is this considered a security issue? This crosses no security boundary, it only lacks sanity handling for broken use of a function?

@mmuehlenhoff FWIW, I do not know, I'm not related with requesting the CVE, I was just relaying it here after doing some CVE triage in a downstream distribution. It might be sensible to ask the assigning CNA for rejection if the issue is not considered valid security issue.

DaveGamble / cJSON