yyjson_read_opts crashses if the allocator pool is too small

[andrei-datcu]

Describe the bug full repro

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

int main()
{
        char json[] = "{\"alg\":\"ES256\",\"typ\":\"JWT\"}\0\0\0\0\0";
        char BUF[32]; // change me to >= 64 and I won't crash
        yyjson_alc json_alc;
        yyjson_alc_pool_init(&json_alc, BUF, sizeof(BUF));

        yyjson_doc* doc = yyjson_read_opts(json, strlen(json), YYJSON_READ_INSITU, &json_alc, NULL);
        if (!doc) {
                printf("OOM\n");
                return -1;
        }
        const char* alg = yyjson_get_str(yyjson_obj_get(yyjson_doc_get_root(doc), "alg"));
        printf("%s\n", alg);
        return 0;
}

Stack:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x0000000000429c76 in read_root_minify (err=0x7fffffffa130, flg=1, alc=..., end=0x7fffffffd61b "", cur=0x7fffffffd600 "{\"alg\":\"ES256\",\"typ\":\"JWT\"}", hdr=0x7fffffffd600 "{\"alg\":\"ES256\",\"typ\":\"JWT\"}") at src/yyjson.c:4705
#2  yyjson_read_opts (dat=0x7fffffffd600 "{\"alg\":\"ES256\",\"typ\":\"JWT\"}", len=27, flg=1, alc_ptr=0x7fffffffd5c0, err=0x7fffffffa130) at src/yyjson.c:5535
#3  0x00000000004012ce in main () at crash.c:12

If BUF's size is 64 then we correctly get a NULL document.

Your environment

• OS: openSuse Tumbleweed
• Compiler: gcc 12.1.1

Additional context Reproduces on ee9e6d5d3bde4a26b78e9cfd596dd28d231266fa

[ibireme]

As mentioned in the documentation: if initialization fails, yyjson_alc_pool_init() returns false and alc is left unmodified.

[andrei-datcu]

Sorry for missing that.

[ibireme]

It looks like this could cause a crash if user doesn't check the return value of yyjson_alc_pool_init().

So I modified this function: when initialization fails, use a set of placeholder functions to initialize alc, see: https://github.com/ibireme/yyjson/commit/72f19e2d455bbfaaf41c933a8bfc74354ac61823

[TkTech]

Just my opinion, but I disagree with this change. The user is responsible for checking the allocation worked, and that's clearly documented. I'm using my custom allocator for a reason, I don't want a different allocator suddenly involved. For example in the python wrapper this would mean we suddenly lose tracing of memory.

[ibireme]

@TkTech This only involves allocators initialized with yyjson_alc_pool_init(), not other custom allocators. So I think it's fine.

// A simple example:
// buffer is not big enough to create a `pool allocator`
char buf[1];
yyjson_alc alc;
yyjson_alc_pool_init(&alc, buf, sizeof(buf));

// before: crash due to 'malloc' function pointer not being set
// after: just return NULL
void *mem = alc.malloc(alc.ctx, 64);

[TkTech]

Thanks @ibireme, I understand now :)