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

cJSON_AddItemToObject does not have a way to detect OOM from add_item_to_object.cJSON_strdup #435

Closed
mkmoisen opened this issue Mar 1, 2020 · 2 comments

Comments

@mkmoisen
Copy link

mkmoisen commented Mar 1, 2020

cJSON_AddItemToObject is function returning void.

It calls add_item_to_object, which is a function returning cJSON_bool.

add_item_to_object calls cJSON_strdup here, which can fail if there is no memory left. Under this condition, add_item_to_object returns false, having never added the item to the object.

Finally, since cJSON_AddItemToObject returns NULL, the calling code cannot check for this condition.

I found this while trying to add test coverage in my code for out of memory errors, by manipulating the malloc_fn function via cJSON_InitHooks.

You can prove it like the following:

#include "cJSON.h"
#include <stdio.h>
#include <stdlib.h>

int times_without_failing = 0;
int times_before_failing = 0;

void *test_malloc_fail(size_t size)
{
    if (times_without_failing++ == times_before_failing) {
        printf("malloc reutrning none\n");
        return NULL;
    }
    printf("malloc returning real\n");
    return malloc(size);
}

void test_hooks(int i) {
    cJSON_Hooks hooks;

    hooks.malloc_fn = (void *) test_malloc_fail;
    hooks.free_fn = (void *) free;
    times_without_failing = 0;
    times_before_failing = i;
    cJSON_InitHooks(&hooks);

}

int main(void)
{
    test_hooks(2);
    cJSON *json = cJSON_CreateObject();
    cJSON *object = cJSON_CreateObject();
    cJSON_AddItemToObject(json, "foo", object);
    if (cJSON_HasObjectItem(json, "foo")) {
        printf("foo is in json\n");
    } else {
        printf("foo is not in json\n");
    }
    printf("done\n");
}

This prints out:

malloc returning real
malloc returning real
malloc reutrning none
foo is not in json
done
@Alanscut
Copy link
Collaborator

hi @mkmoisen , I totally agree with that. Many isssues are related with the memory allocation failure, but if all the function with memory allocation will return a flag or throws an exception when memory allocation failure, one could found the problem timely.
My suggestion is to change the return variable from void to boolean, true - adding successfully, false - adding failed. What do you think?

@Alanscut
Copy link
Collaborator

Alanscut commented Apr 3, 2020

closed for #453

@Alanscut Alanscut closed this as completed Apr 3, 2020
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