Theoretical null pointer dereference in cJSONUtils_InplaceDecodePointerString() from cJSONUtils_PatchDetach()

[bnason-nf]

Hi,

I ran the clang static analyzer on cJSON and it found this potential issue:

Event 1: Logic error: Dereference of null pointer (loaded from variable 'string') (3rdparty/cjson/cjson_utils.c:211)
3rdparty/cjson/cjson_utils.c

   197            }
   198            else
   199            {
   200                return NULL;
   201            }
   202        }
   203    
   204        return object;
   205    }
   206    
   207    /* JSON Patch implementation. */
   208    static void cJSONUtils_InplaceDecodePointerString(char *string)
   209    {
   210        char *s2 = string;
   211        for (; *string; s2++, string++)
                     ^ Logic error: Dereference of null pointer (loaded from variable 'string')
   212        {
   213            *s2 = (*string != '~')
   214                ? (*string)
   215                : ((*(++string) == '0')
   216                        ? '~'
   217                        : '/');
   218        }
   219    
   220        *s2 = '\0';
   221    }
   222    
   223    static cJSON *cJSONUtils_PatchDetach(cJSON *object, const char *path)
   224    {
   225        char *parentptr = NULL;
 

Value assigned to 'childptr'
3rdparty/cjson/cjson_utils.c

   229    
   230        /* copy path and split it in parent and child */
   231        parentptr = cJSONUtils_strdup(path);
   232        childptr = strrchr(parentptr, '/'); /* last '/' */
              ^ Value assigned to 'childptr'
   233        if (childptr)
   234        {
   235            /* split strings */
 
Assuming 'childptr' is null
3rdparty/cjson/cjson_utils.c

   230        /* copy path and split it in parent and child */
   231        parentptr = cJSONUtils_strdup(path);
   232        childptr = strrchr(parentptr, '/'); /* last '/' */
   233        if (childptr)
                  ^ Assuming 'childptr' is null
   234        {
   235            /* split strings */
   236            *childptr++ = '\0';
 
Passing null pointer value via 1st parameter 'string'
3rdparty/cjson/cjson_utils.c

   236            *childptr++ = '\0';
   237        }
   238        parent = cJSONUtils_GetPointer(object, parentptr);
   239        cJSONUtils_InplaceDecodePointerString(childptr);
                                                    ^ Passing null pointer value via 1st parameter 'string'
   240    
   241        if (!parent)
   242        {
 
Calling 'cJSONUtils_InplaceDecodePointerString'
3rdparty/cjson/cjson_utils.c

   236            *childptr++ = '\0';
   237        }
   238        parent = cJSONUtils_GetPointer(object, parentptr);
   239        cJSONUtils_InplaceDecodePointerString(childptr);
              ^ Calling 'cJSONUtils_InplaceDecodePointerString'
   240    
   241        if (!parent)
   242        {
 
Entered call from 'cJSONUtils_PatchDetach'
3rdparty/cjson/cjson_utils.c

   205    }
   206    
   207    /* JSON Patch implementation. */
   208    static void cJSONUtils_InplaceDecodePointerString(char *string)
          ^ Entered call from 'cJSONUtils_PatchDetach'
   209    {
   210        char *s2 = string;
   211        for (; *string; s2++, string++)
 
Dereference of null pointer (loaded from variable 'string')
3rdparty/cjson/cjson_utils.c

   208    static void cJSONUtils_InplaceDecodePointerString(char *string)
   209    {
   210        char *s2 = string;
   211        for (; *string; s2++, string++)
                     ^ Dereference of null pointer (loaded from variable 'string')
   212        {
   213            *s2 = (*string != '~')
   214                ? (*string)

This looks trivial to address with a strategic null pointer check.

Thanks,
Benbuck

[FSMaxB]

Thanks for reporting this. A fix is on the way.

[bnason-nf]

Thanks, very impressive turn around time on that.

-Benbuck