fix insufficient check in cJSON_DetachItemViaPointer#722
Conversation
cJSON_DetachItemViaPointer() will crash if the detached item has field `prev` is null. The common suitation scenario is the detached item is created by cJSON_Create* APIs and directly pass it to cJSON_DetachItemViaPointer(object, item) call without adding item to object previously. Then the cJSON_DetachItemViaPointer() will crash because it does not check whether the passed item has valid `prev` field. As detach a non-existent item is an undesirable behavior, instead of raising an uneasy core dump, this commit adds the NULL check of `item->prev` in cJSON_DetachItemViaPointer and return NULL to inform user such unexpect behavior (as user will routinely free/handle the detached resources later). Signed-off-by: hopper-vul <[email protected]>
|
The smallest case is |
|
I think the null check of Just think about this scenario: |
| CJSON_PUBLIC(cJSON *) cJSON_DetachItemViaPointer(cJSON *parent, cJSON * const item) | ||
| { | ||
| if ((parent == NULL) || (item == NULL)) | ||
| if ((parent == NULL) || (item == NULL) || (item->prev == NULL)) |
There was a problem hiding this comment.
I prefer adding this null check to the place where item->prev is used instead of here.
I mean:
if (item != parent->child)
{
if (item->prev == NULL)
{
return NULL;
}
/* not the first element */
item->prev->next = item->next;
}
|
3 participants
cJSON_DetachItemViaPointer() will crash if the detached item has field
previs null. The common suitation scenario is the detached item is created by cJSON_Create* APIs and directly pass it to cJSON_DetachItemViaPointer(object, item) call without adding item to object previously. Then the cJSON_DetachItemViaPointer() will crash because it does not check whether the passed item has validprevfield.As detach a non-existent item is an undesirable behavior, instead of raising an uneasy core dump, this commit adds the NULL check of
item->previn cJSON_DetachItemViaPointer and return NULL to inform user such unexpect behavior (as user will routinely free/handle the detached resources later).Signed-off-by: hopper-vul [email protected]