String deallocated before use

[hhallen]

Maybe I'm doing things in a dumb way, but given the method I'm using, there's a problem with the function add_item_to_object(). What I want to do is to parse a number of separate .json files and merge their contents into a single cJSON object. The files may partially duplicate each other. Objects and array items that are already in the result that is being constructed should be skipped during the processing of a .json file, i.e. no duplicate objects or array elements are permitted in the result output.

So, at the top level each file contains one object (always with the same name, "stuff" in the simplified examples below), which in turn contains other objects, which contain arrays of objects. A simple example will illustrate what I want to achieve.

.json file 1:

{
    "stuff": {
        "FOO": [
            {
                "name": "foo_1",
                "value": "11111111"
            },
            {
                "name": "foo_2",
                "value": "11111112"
            }
        ],
        "BAR": [
            {
                "name": "bar_1",
                "value": "22222221"
            },
            {
                "name": "bar_2",
                "value": "22222222"
            }
        ]
    }
}

.json file 2:

{
    "stuff": {
        "FOO": [
            {
                "name": "foo_1",
                "value": "11111111"
            },
            {
                "name": "foo_3",
                "value": "11111113"
            }
        ],
        "BAZ": [
            {
                "name": "baz_1",
                "value": "33333331"
            }
        ]
    }
}

I want the result of the merge operation to be a cJSON object that represents the following .json:

{
    "stuff": {
        "FOO": [
            {
                "name": "foo_1",
                "value": "11111111"
            },
            {
                "name": "foo_2",
                "value": "11111112"
            },
            {
                "name": "foo_3",
                "value": "11111113"
            }
        ],
        "BAR": [
            {
                "name": "bar_1",
                "value": "22222221"
            },
            {
                "name": "bar_2",
                "value": "22222222"
            }
        ],
        "BAZ": [
            {
                "name": "baz_1",
                "value": "33333331"
            }
        ]
    }
}

First, I use cJSON_CreateObject() to start my result object. Then I process each .json file in turn, using cJSON_Parse() to create a cJSON object that represents the file. Then this cJSON object is examined, and if any "stuff" is found, it is moved to the result object, except such objects/array elements as are already present there.

Here is a code snippet that shows how I handle the merge operation for an object at the level directly under "stuff" that is not yet present at all in the result:

            newGroup = cJSON_DetachItemFromObject(newStuff, newGroup->string);
            cJSON_AddItemToObject(resultStuff, newGroup->string, newGroup);

newGroup is a cJSON object from the current .json file, representing for example "BAZ" from the .json files above. I want to move that object from the cJSON object representing the .json file (newStuff) to the cJSON result object under construction (resultStuff).

The problem is that the internal function add_item_to_object() in cJSON assumes that its string argument is valid throughout the function. But when I use newGroup->string, that assumption becomes false, because that string is deallocated before a copy is made and inserted in the item before it's added to the target object. The result (at best) is that the item string will contain garbage.

Perhaps the order of actions in add_item_to_object() could be rearranged, so that a copy of the string argument is made before any deallocations?

Of course, I could allocate memory and make a copy of the string in my own code, but that seems a bit silly. Maybe my method is naive, and a there is a better way of doing what I want that does not run into this kind of problem? If so, please let me know. I'm relatively new to cJSON.

[FSMaxB]

Maybe cJSONUtils_MergePatchCaseSensitive does what you want. It implements RFC 7396. In this case your objects mustn't contain null though.

Otherwise, use cJSON_DetachItemViaPointer using the newGroup pointer. That will make the code faster because it doesn't need to iterate over all the object items anymore.

Also you can use cJSON_AddItemToArray instead of cJSON_AddItemToObject to get around the use after free poblem. This also avoids the cJSON_strdup.

I still think that it makes sense though to rearrange the order in which the values are accessed. It just never occured to me before that the parameter to cJSON_AddItemToObject could be an alias of the items string property. So I guess I will fix that and write a test for it, so I don't accidentally unfix it.

[FSMaxB]

Since it is a mistake that could easily happen and there is no warning about it, I consider this a bug. And because of the use-after-free it can potentially have security implications.

I will fix this in a new bugfix release as soon as possible.

[FSMaxB]

The use after free is now fixed in 1.7.4.

This can be closed as soon as your use case of merging multiple JSON objects is figured out.

[hhallen]

Thanks for the tips in your first reply, and for fixing the issue so quickly. Unfortunately the 'merge patch' method doesn't seem to quite do what I need (the actual use case is a bit more complex than the example I gave). But the tips about cJSON_DetachItemViaPointer and cJSON_AddItemToArray were useful.