fix insufficient check in cJSON_DetachItemViaPointer

[hopper-vul]

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]

[hopper-vul]

The smallest case is

#include "cJSON.h"
  
int main(){
        cJSON *object = cJSON_CreateObject();
        cJSON *item = cJSON_CreateObject();
        cJSON *deatch = cJSON_DetachItemViaPointer(object, item); //core dump!
        if (deatch == NULL)
                cJSON_Delete(item);
        cJSON_Delete(object);
        return 0;
}

[hopper-vul]

@Alanscut @FSMaxB Hi, could you please review this PR? I'm looking forward to your feedback.

Thanks.

[Alanscut]

I think the null check of item-prev should be at the place it's visited.

Just think about this scenario: When item == parent->child, it means item is the first child of parent. In this case, the item->prev is NULL cause it's the first item. In your implementation this will return NULL and it's not what we are expected.