Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange behavior observed in cJSON_ReplaceItemInArray #821

Open
tregua87 opened this issue Jan 15, 2024 · 1 comment
Open

Strange behavior observed in cJSON_ReplaceItemInArray #821

tregua87 opened this issue Jan 15, 2024 · 1 comment

Comments

@tregua87
Copy link

tregua87 commented Jan 15, 2024

I encountered this strange behavior in the API cJSON_ReplaceItemInArray.
Here below I paste a minimal example and an explanation.
I replicated this error in the latest cJSON commit 87d8f0961a01bf09bef98ff89bae9fdec42181ee, and compiling with these options:

clang   -g -std=c  11 -I$INSTALL_DIR/include -fsanitize=address \
		test.c -Wl,--whole-archive $INSTALL_DIR/libcjson.a -Wl,--no-whole-archive \
		-lz -ljpeg -Wl,-Bstatic -llzma -Wl,-Bdynamic -lstdc   -o test

(some options might be redundant , sorry)

The issue appears when trying to replace an item with itself: cJSON_ReplaceItemInArray(json, 0, json).
What I observe is that the library generates a loop in the object graphs, thus making the final object unuseful, i.e., item[0] -> item
One of the consequence is that the cJSON_Delete produces stack exhaustion.
This is alarming because the return value ret_v is 1, thus the program assumes the replacement was correct.

Even though I understand this might not be the intended usage of cJSON_ReplaceItemInArray, we should be able to detect loops in object graphs.
Another approach is to generate a copy for the newitem argument for cJSON_ReplaceItemInArray.
This will avoid recursions, but it might lead to memory leaks.

#include <cjson/cJSON.h>

#include <string.h>
#include <stdio.h>

int main(int argn, char** argc) {

    int ret_v;
    cJSON *json;
    
    char *x = "[true]";
    
    printf("start\n");

    json =  cJSON_ParseWithLength(x, strlen(x) 1);
    if (json == 0) {
        printf("error 1\n");
        return 0;
    }

    printf("json: %s\n", cJSON_Print(json));

    ret_v =  cJSON_ReplaceItemInArray(json, 0, json);
    if (ret_v == 0) 
        printf("Replace failed!\n");
    else
        printf("Replaced success!!\n");
    
    printf("end\n");

    cJSON_Delete(json);

    return 0;
}
@daschfg
Copy link

daschfg commented Jan 26, 2024

So with the call to cJSON_ReplaceItemInArray, you're replacing the first entry in the array with a reference to the whole array, thus creating a circular reference.

This is somewhat similar to #807 and probably would require a similar solution.

I expect generating a copy for every insert and replace operation would not be possible without a breaking change in the API.
Checking the whole object for duplicates on the other hand would probably lead to performance issues down the road, so I'm not sure about the best solution on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants