From f683f06082cc6d7de1ace298f84dfa813186753f Mon Sep 17 00:00:00 2001 From: Ugur Guenduez Date: Thu, 20 Aug 2026 12:41:08 +0200 Subject: [PATCH] pack: bound msgpack2json() recursion depth to prevent stack overflow msgpack2json() in flb_pack.c recurses once per nesting level while converting a msgpack object into a JSON string, with no limit on how deep it will go for MSGPACK_OBJECT_ARRAY and MSGPACK_OBJECT_MAP values. A msgpack_object tree that is nested deeply enough (for example one built from a deeply nested JSON log line by a JSON-to-msgpack conversion elsewhere in the pipeline, which is not bound by the wire-level unpacker's own MSGPACK_EMBED_STACK_SIZE guard) will recurse past the available stack and crash the process with SIGSEGV. This was observed in production: the out_loki output plugin calls flb_msgpack_to_json_str() -> flb_msgpack_to_json() -> msgpack2json() from pack_record() while flushing records, and a deeply nested record reliably crashed the process with a stack overflow inside msgpack2json(), taking down the whole Fluent Bit instance and losing the in-flight flush batch. Add a FLB_PACK_JSON_MAX_DEPTH (512) guard: once msgpack2json() would recurse past this depth, stop descending and encode the remaining structure as a JSON null instead, so the conversion still completes and returns a valid (truncated) JSON string rather than crashing. 512 levels is far beyond any reasonably structured log record while keeping stack usage negligible. A non-empty map whose key/value pairs would land exactly one level past the limit needs special care: a JSON object key must always be a quoted string, so truncating an individual key to a bare null would produce invalid JSON such as {null:...}. Both MSGPACK_OBJECT_ARRAY and MSGPACK_OBJECT_MAP now check depth one level ahead of opening their bracket/brace, and render the whole container as null instead of partially opening it when their children would exceed the limit. The truncation warning is now logged at most once per top-level conversion (via a warned flag threaded through the recursion) instead of once per truncated branch, to avoid flooding the log when a single record has many separate over-depth branches. The msgpack_object *p pointer used while walking an array's elements is now declared with the function's other local variables instead of inside the loop != 0 block, matching this project's coding style of declaring variables at the top of the function; only the assignment stays inside the conditional. Verified locally with a small reproduction harness that builds an in-memory msgpack_object tree several hundred thousand levels deep and calls flb_msgpack_to_json_str() directly: - Before this patch: SIGSEGV (stack overflow), matching the exact production crash signature (out_loki's pack_record() call site). - After this patch: the call returns a valid, truncated JSON string and the process does not crash. Added two regression tests to tests/internal/pack.c: json_pack_deep_map_boundary builds a non-empty map exactly at the 512-level boundary and checks the exact output shape (length, and the null literal starting at the exact offset right after the opening brackets) instead of just checking that a null appears somewhere in the output; json_pack_deep_map_below_boundary builds the same map shape comfortably under the limit and checks its real content is still serialized. Both tests declare their local variables (including out_len and expected_len) at the top of the function per this project's coding style. Full tests/internal/pack.c suite (27 tests) passes, and the two new tests run clean under Valgrind (0 errors, 0 leaks, all blocks freed). Signed-off-by: Ugur Guenduez --- src/flb_pack.c | 82 ++++++++++++++++++++-- tests/internal/pack.c | 156 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 231 insertions(+), 7 deletions(-) diff --git a/src/flb_pack.c b/src/flb_pack.c index 581255f328b..66b498a1d82 100644 --- a/src/flb_pack.c +++ b/src/flb_pack.c @@ -51,6 +51,17 @@ #define try_to_write_str flb_utils_write_str +/* + * Maximum recursion depth allowed while converting a msgpack object into a + * JSON string (see msgpack2json() below). Msgpack arrays and maps can be + * nested arbitrarily deep, and msgpack2json() recurses once per nesting + * level. Without a bound, a deeply nested (malformed, corrupted or + * maliciously crafted) record can recurse deep enough to overflow the + * thread stack and crash the process. 512 levels is far beyond any + * reasonably structured log record while keeping stack usage negligible. + */ +#define FLB_PACK_JSON_MAX_DEPTH 512 + static int convert_nan_to_null = FLB_FALSE; static int flb_pack_set_null_as_nan(int b) { @@ -981,14 +992,42 @@ static inline int key_exists_in_map(msgpack_object key, msgpack_object map, int return FLB_FALSE; } +/* + * Log the maximum-nesting-depth truncation warning at most once per + * top-level msgpack2json() conversion, regardless of how many separate + * branches in the structure end up being truncated. + */ +static void msgpack2json_depth_warn(int *warned) +{ + if (warned != NULL && *warned == FLB_FALSE) { + flb_warn("[pack] msgpack to JSON conversion exceeded the maximum " + "nesting depth (%d), truncating remaining structure", + FLB_PACK_JSON_MAX_DEPTH); + *warned = FLB_TRUE; + } +} + static int msgpack2json(char *buf, int *off, size_t left, - const msgpack_object *o, int escape_unicode) + const msgpack_object *o, int escape_unicode, + int depth, int *warned) { int i; int dup; int ret = FLB_FALSE; int loop; int packed; + msgpack_object *p; + + /* + * Stop descending once the maximum nesting depth is reached and encode + * the remaining structure as a JSON null instead of recursing further. + * This keeps the conversion bounded and avoids a stack overflow on + * pathologically nested input, see FLB_PACK_JSON_MAX_DEPTH above. + */ + if (depth > FLB_PACK_JSON_MAX_DEPTH) { + msgpack2json_depth_warn(warned); + return try_to_write(buf, off, left, "null", 4); + } switch(o->type) { case MSGPACK_OBJECT_NIL: @@ -1078,17 +1117,30 @@ static int msgpack2json(char *buf, int *off, size_t left, case MSGPACK_OBJECT_ARRAY: loop = o->via.array.size; + if (loop != 0 && depth + 1 > FLB_PACK_JSON_MAX_DEPTH) { + /* + * The array is non-empty but its elements would exceed the + * maximum nesting depth. Render the whole array as null + * instead of opening it and only then truncating an element, + * keeping the output symmetric with the MSGPACK_OBJECT_MAP + * case below. + */ + msgpack2json_depth_warn(warned); + ret = try_to_write(buf, off, left, "null", 4); + break; + } + if (!try_to_write(buf, off, left, "[", 1)) { goto msg2json_end; } if (loop != 0) { - msgpack_object* p = o->via.array.ptr; - if (!msgpack2json(buf, off, left, p, escape_unicode)) { + p = o->via.array.ptr; + if (!msgpack2json(buf, off, left, p, escape_unicode, depth + 1, warned)) { goto msg2json_end; } for (i=1; ivia.map.size; + + if (loop != 0 && depth + 1 > FLB_PACK_JSON_MAX_DEPTH) { + /* + * The map is non-empty but its keys/values would exceed the + * maximum nesting depth. A JSON object key must always be a + * quoted string; truncating an individual key to a bare + * "null" (as the generic depth guard above would do) produces + * invalid JSON such as {null:...}. Render the whole map as + * null instead of opening it. + */ + msgpack2json_depth_warn(warned); + ret = try_to_write(buf, off, left, "null", 4); + break; + } + if (!try_to_write(buf, off, left, "{", 1)) { goto msg2json_end; } @@ -1124,9 +1191,9 @@ static int msgpack2json(char *buf, int *off, size_t left, } if ( - !msgpack2json(buf, off, left, &(p+i)->key, escape_unicode) || + !msgpack2json(buf, off, left, &(p+i)->key, escape_unicode, depth + 1, warned) || !try_to_write(buf, off, left, ":", 1) || - !msgpack2json(buf, off, left, &(p+i)->val, escape_unicode) ) { + !msgpack2json(buf, off, left, &(p+i)->val, escape_unicode, depth + 1, warned) ) { goto msg2json_end; } packed++; @@ -1158,12 +1225,13 @@ int flb_msgpack_to_json(char *json_str, size_t json_size, { int ret = -1; int off = 0; + int warned = FLB_FALSE; if (json_str == NULL || obj == NULL) { return -1; } - ret = msgpack2json(json_str, &off, json_size - 1, obj, escape_unicode); + ret = msgpack2json(json_str, &off, json_size - 1, obj, escape_unicode, 0, &warned); json_str[off] = '\0'; return ret ? off: ret; } diff --git a/tests/internal/pack.c b/tests/internal/pack.c index 60f79238286..a7f6fe1314b 100644 --- a/tests/internal/pack.c +++ b/tests/internal/pack.c @@ -1077,6 +1077,160 @@ void test_json_pack_bug5336() } /* Ensure empty arrays inside nested objects are handled */ +/* + * Must mirror FLB_PACK_JSON_MAX_DEPTH in src/flb_pack.c (not exposed via a + * public header since it is an internal implementation detail of + * msgpack2json()). + */ +#define TEST_PACK_MAX_DEPTH 512 + +/* + * Regression test: a non-empty msgpack map whose key/value pairs would be + * evaluated exactly one level past FLB_PACK_JSON_MAX_DEPTH must still be + * rendered as valid JSON. The whole map is expected to collapse to a JSON + * null literal instead of emitting a bare, unquoted null in place of a map + * key (which would produce invalid JSON such as {null:"v"}). + */ +void test_json_pack_deep_map_boundary() +{ + int i; + char *out; + char *p; + msgpack_object *chain; + msgpack_object_kv kv; + size_t out_len; + size_t expected_len; + + /* + * Build TEST_PACK_MAX_DEPTH nested single-element arrays with a + * non-empty map ({"k":"v"}) as the innermost element, constructed + * directly in memory. chain[0] is evaluated at depth 0, chain[i] at + * depth i, so the map at chain[TEST_PACK_MAX_DEPTH] is evaluated at + * depth == TEST_PACK_MAX_DEPTH: its own guard passes, but its key/value + * pair would be one level past the limit. + */ + chain = flb_malloc(sizeof(msgpack_object) * (TEST_PACK_MAX_DEPTH + 1)); + if (!TEST_CHECK(chain != NULL)) { + TEST_MSG("could not allocate test msgpack_object chain"); + return; + } + + for (i = 0; i < TEST_PACK_MAX_DEPTH; i++) { + chain[i].type = MSGPACK_OBJECT_ARRAY; + chain[i].via.array.size = 1; + chain[i].via.array.ptr = &chain[i + 1]; + } + + kv.key.type = MSGPACK_OBJECT_STR; + kv.key.via.str.size = 1; + kv.key.via.str.ptr = "k"; + kv.val.type = MSGPACK_OBJECT_STR; + kv.val.via.str.size = 1; + kv.val.via.str.ptr = "v"; + + chain[TEST_PACK_MAX_DEPTH].type = MSGPACK_OBJECT_MAP; + chain[TEST_PACK_MAX_DEPTH].via.map.size = 1; + chain[TEST_PACK_MAX_DEPTH].via.map.ptr = &kv; + + out = flb_msgpack_to_json_str(1024, &chain[0], FLB_FALSE); + flb_free(chain); + + if (!TEST_CHECK(out != NULL)) { + TEST_MSG("flb_msgpack_to_json_str returned NULL"); + return; + } + + /* a map key must never be truncated to an unquoted null */ + p = strstr(out, "null:"); + if (!TEST_CHECK(p == NULL)) { + TEST_MSG("map key was rendered as an unquoted null: %s", out); + } + + /* + * Exact shape check: TEST_PACK_MAX_DEPTH opening brackets, then the + * truncated map as a bare "null", then TEST_PACK_MAX_DEPTH closing + * brackets. Verify both the total length and that the null literal + * begins exactly at offset TEST_PACK_MAX_DEPTH, rather than accepting + * "null" anywhere in the output. + */ + { + out_len = strlen(out); + expected_len = (size_t) TEST_PACK_MAX_DEPTH * 2 + 4; + + if (!TEST_CHECK(out_len == expected_len)) { + TEST_MSG("unexpected output length: expected=%zu got=%zu out=%s", + expected_len, out_len, out); + } + + if (!TEST_CHECK(out_len > (size_t) TEST_PACK_MAX_DEPTH + 4 && + strncmp(out + TEST_PACK_MAX_DEPTH, "null", 4) == 0)) { + TEST_MSG("expected a null literal at offset %d: %s", + TEST_PACK_MAX_DEPTH, out); + } + } + + /* the original key/value content must not appear: it was truncated */ + p = strstr(out, "\"k\":\"v\""); + if (!TEST_CHECK(p == NULL)) { + TEST_MSG("map content should have been truncated: %s", out); + } + + flb_free(out); +} + +/* + * Companion check: the same map shape placed comfortably below the depth + * limit must still serialize its real content (i.e. the pre-check added for + * the boundary case above must not fire early for valid, shallower input). + */ +void test_json_pack_deep_map_below_boundary() +{ + int i; + int shallow_depth = TEST_PACK_MAX_DEPTH - 5; + char *out; + char *p; + msgpack_object *chain; + msgpack_object_kv kv; + + chain = flb_malloc(sizeof(msgpack_object) * (shallow_depth + 1)); + if (!TEST_CHECK(chain != NULL)) { + TEST_MSG("could not allocate test msgpack_object chain"); + return; + } + + for (i = 0; i < shallow_depth; i++) { + chain[i].type = MSGPACK_OBJECT_ARRAY; + chain[i].via.array.size = 1; + chain[i].via.array.ptr = &chain[i + 1]; + } + + kv.key.type = MSGPACK_OBJECT_STR; + kv.key.via.str.size = 1; + kv.key.via.str.ptr = "k"; + kv.val.type = MSGPACK_OBJECT_STR; + kv.val.via.str.size = 1; + kv.val.via.str.ptr = "v"; + + chain[shallow_depth].type = MSGPACK_OBJECT_MAP; + chain[shallow_depth].via.map.size = 1; + chain[shallow_depth].via.map.ptr = &kv; + + out = flb_msgpack_to_json_str(1024, &chain[0], FLB_FALSE); + flb_free(chain); + + if (!TEST_CHECK(out != NULL)) { + TEST_MSG("flb_msgpack_to_json_str returned NULL"); + return; + } + + p = strstr(out, "\"k\":\"v\""); + if (!TEST_CHECK(p != NULL)) { + TEST_MSG("map content below the depth limit should be preserved: %s", out); + } + + flb_free(out); +} + void test_json_pack_empty_array() { int ret; @@ -1305,6 +1459,8 @@ TEST_LIST = { { "json_pack_nan" , test_json_pack_nan}, { "json_pack_bug5336" , test_json_pack_bug5336}, { "json_pack_empty_array", test_json_pack_empty_array}, + { "json_pack_deep_map_boundary", test_json_pack_deep_map_boundary}, + { "json_pack_deep_map_below_boundary", test_json_pack_deep_map_below_boundary}, { "json_date_iso8601" , test_json_date_iso8601}, { "json_date_double" , test_json_date_double}, { "json_date_java_sql" , test_json_date_java_sql},