From d41abbfeb94adc5d0130266b8e8a5f21aab396cd Mon Sep 17 00:00:00 2001 From: Balint Uveges Date: Mon, 17 Aug 2026 20:47:45 +0200 Subject: [PATCH] printer cbor BUGFIX crash when printing (leaf-)lists nested in a list The CBOR printer kept a single "currently open array" pointer (cborpr_ctx.array) shared across all nesting levels, while tracking the open nodes on a proper stack (cborpr_ctx.open). When a leaf-list or list was nested inside a list, printing the inner array overwrote that pointer and then reset it to NULL, so the enclosing list subsequently pushed into a NULL array, causing a segfault in cbor_array_push(). Replace the single array pointer with a stack (cborpr_ctx.arrays) kept parallel to the open-node stack, add cbor_current_array() to fetch the innermost open array, and update cbor_print_leaf_list()/cbor_print_opaq() and their callers accordingly. The stack is freed on all exit paths of cbor_print_data(). Add a regression test (test_nested_list) covering a leaf-list nested in a list, a list nested in a list, and mixed multi-instance nesting, plus the supporting list-nested node in the cbor-test module. Co-authored-by: Cursor --- src/printer_cbor.c | 80 ++++++++++++++++++++++--------- tests/modules/yang/cbor-test.yang | 31 ++++++++++++ tests/utests/data/test_cbor.c | 67 ++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 22 deletions(-) diff --git a/src/printer_cbor.c b/src/printer_cbor.c index c53b9c2f2..49a1a3c31 100644 --- a/src/printer_cbor.c +++ b/src/printer_cbor.c @@ -51,7 +51,8 @@ struct cborpr_ctx { const struct lyd_node *first_leaflist; /**< first printed leaf-list instance, used when printing its metadata/attributes */ cbor_item_t *root_map; /**< root CBOR map */ - cbor_item_t *array; /**< currently open CBOR array for leaf-list/list instances */ + cbor_item_t **arrays; /**< stack of currently open CBOR arrays, parallel to `open` (one per nesting level) */ + uint32_t arrays_size; /**< allocated size of the `arrays` stack */ }; static LY_ERR cbor_print_node(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *parent_map); @@ -93,12 +94,39 @@ matching_node(const struct lyd_node *node1, const struct lyd_node *node2) * @return LY_ERR value. */ static LY_ERR -cbor_print_array_open(struct cborpr_ctx *pctx, const struct lyd_node *node) +cbor_print_array_open(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *array) { LY_CHECK_RET(ly_set_add(&pctx->open, (void *)node, 0, NULL)); + + /* keep the array pointer on a stack parallel to `open` so that nested + * (leaf-)lists each have their own open array instead of a single shared one */ + if (pctx->open.count > pctx->arrays_size) { + cbor_item_t **tmp = realloc(pctx->arrays, pctx->open.count * sizeof *pctx->arrays); + + LY_CHECK_RET(!tmp, LY_EMEM); + pctx->arrays = tmp; + pctx->arrays_size = pctx->open.count; + } + pctx->arrays[pctx->open.count - 1] = array; + return LY_SUCCESS; } +/** + * @brief Get the innermost currently open CBOR array. + * + * @param[in] pctx CBOR printer context. + * @return The innermost open array, or NULL if none is open. + */ +static cbor_item_t * +cbor_current_array(struct cborpr_ctx *pctx) +{ + if (!pctx->open.count) { + return NULL; + } + return pctx->arrays[pctx->open.count - 1]; +} + /** * @brief Get know if the array for the provided @p node is currently open. * @@ -922,31 +950,35 @@ cbor_print_array_is_last_inst(struct cborpr_ctx *pctx, const struct lyd_node *no * @return LY_ERR value. */ static LY_ERR -cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *parent_map, cbor_item_t **array_p) +cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_t *parent_map) { const struct lys_module *wdmod = NULL; cbor_item_t *value_item = NULL; cbor_item_t *inner_map = NULL; + cbor_item_t *array; char *key = NULL; if (!is_open_array(pctx, node)) { /* start new array */ - *array_p = cbor_new_indefinite_array(); - LY_CHECK_RET(!*array_p, LY_EMEM); + cbor_item_t *new_array = cbor_new_indefinite_array(); - LY_CHECK_RET(cbor_print_array_open(pctx, node)); + LY_CHECK_RET(!new_array, LY_EMEM); + LY_CHECK_ERR_RET(cbor_print_array_open(pctx, node, new_array), cbor_decref(&new_array), LY_EINVAL); } + /* the array for this (leaf-)list is the innermost open one */ + array = cbor_current_array(pctx); if (node->schema->nodetype == LYS_LIST) { /* print list's content */ LY_CHECK_RET(cbor_print_inner(pctx, node, &inner_map)); - LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, inner_map)); + /* re-read: nested (leaf-)lists may have pushed/popped their own arrays */ + LY_CHECK_RET(cbor_array_push_check(pctx->ctx, cbor_current_array(pctx), inner_map)); } else { assert(node->schema->nodetype == LYS_LEAFLIST); LY_CHECK_RET(cbor_print_value(pctx, LYD_CTX(node), &((const struct lyd_node_term *)node)->value, node->schema->module, &value_item)); - LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, value_item)); + LY_CHECK_RET(cbor_array_push_check(pctx->ctx, array, value_item)); if (!pctx->first_leaflist) { if (((node->flags & LYD_DEFAULT) && (pctx->options & (LYD_PRINT_WD_ALL_TAG | LYD_PRINT_WD_IMPL_TAG))) || @@ -962,12 +994,13 @@ cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_ } if (cbor_print_array_is_last_inst(pctx, node)) { + array = cbor_current_array(pctx); key = cbor_print_member_name(pctx, node, 0); /* add completed array to parent map */ if (key) { struct cbor_pair pair = { .key = cbor_move(cbor_build_string(key)), - .value = cbor_move(*array_p) + .value = cbor_move(array) }; free(key); @@ -978,11 +1011,11 @@ cbor_print_leaf_list(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_ return LY_EMEM; } } else { - cbor_decref(array_p); + cbor_decref(&array); + cbor_print_array_close(pctx); return LY_EMEM; } cbor_print_array_close(pctx); - *array_p = NULL; } return LY_SUCCESS; @@ -1091,7 +1124,7 @@ cbor_print_meta_attr_leaflist(struct cborpr_ctx *pctx, cbor_item_t *parent_map) * @return LY_ERR value. */ static LY_ERR -cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_item_t *parent_map, cbor_item_t **array_p) +cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_item_t *parent_map) { ly_bool first = 1, last = 1; uint32_t hints; @@ -1119,9 +1152,11 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_ LY_CHECK_RET(!key, LY_EMEM); if (hints & (LYD_NODEHINT_LIST | LYD_NODEHINT_LEAFLIST)) { - *array_p = cbor_new_indefinite_array(); - LY_CHECK_ERR_RET(!*array_p, free(key), LY_EMEM); - LY_CHECK_ERR_RET(cbor_print_array_open(pctx, &node->node), free(key), LY_EINVAL); + cbor_item_t *new_array = cbor_new_indefinite_array(); + + LY_CHECK_ERR_RET(!new_array, free(key), LY_EMEM); + LY_CHECK_ERR_RET(cbor_print_array_open(pctx, &node->node, new_array), + cbor_decref(&new_array); free(key), LY_EINVAL); } } @@ -1131,7 +1166,7 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_ LY_CHECK_ERR_RET(cbor_print_inner(pctx, &node->node, &inner_map), free(key), LY_EINVAL); if (hints & (LYD_NODEHINT_LIST | LYD_NODEHINT_LEAFLIST)) { - LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, inner_map)); + LY_CHECK_RET(cbor_array_push_check(pctx->ctx, cbor_current_array(pctx), inner_map)); } else { struct cbor_pair pair = { .key = cbor_move(cbor_build_string(key)), @@ -1176,7 +1211,7 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_ LY_CHECK_ERR_RET(!value_item, free(key), LY_EMEM); if (hints & (LYD_NODEHINT_LIST | LYD_NODEHINT_LEAFLIST)) { - LY_CHECK_RET(cbor_array_push_check(pctx->ctx, *array_p, value_item)); + LY_CHECK_RET(cbor_array_push_check(pctx->ctx, cbor_current_array(pctx), value_item)); } else { struct cbor_pair pair = { .key = cbor_move(cbor_build_string(key)), @@ -1206,7 +1241,7 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_ if (key) { struct cbor_pair pair = { .key = cbor_move(cbor_build_string(key)), - .value = cbor_move(*array_p) + .value = cbor_move(cbor_current_array(pctx)) }; free(key); @@ -1218,7 +1253,6 @@ cbor_print_opaq(struct cborpr_ctx *pctx, const struct lyd_node_opaq *node, cbor_ } } cbor_print_array_close(pctx); - *array_p = NULL; } if (key) { @@ -1248,7 +1282,7 @@ cbor_print_node(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_ } if (!node->schema) { - LY_CHECK_RET(cbor_print_opaq(pctx, (const struct lyd_node_opaq *)node, parent_map, &pctx->array)); + LY_CHECK_RET(cbor_print_opaq(pctx, (const struct lyd_node_opaq *)node, parent_map)); } else { switch (node->schema->nodetype) { case LYS_RPC: @@ -1262,7 +1296,7 @@ cbor_print_node(struct cborpr_ctx *pctx, const struct lyd_node *node, cbor_item_ break; case LYS_LEAFLIST: case LYS_LIST: - LY_CHECK_RET(cbor_print_leaf_list(pctx, node, parent_map, &pctx->array)); + LY_CHECK_RET(cbor_print_leaf_list(pctx, node, parent_map)); break; case LYS_ANYDATA: case LYS_ANYXML: @@ -1322,7 +1356,7 @@ cbor_print_data(struct ly_out *out, const struct lyd_node *root, uint32_t option LY_LIST_FOR(root, node) { pctx.root = node; LY_CHECK_ERR_RET(cbor_print_node(&pctx, node, pctx.root_map), - cbor_decref(&pctx.root_map); ly_set_erase(&pctx.open, NULL), LY_EINVAL); + cbor_decref(&pctx.root_map); ly_set_erase(&pctx.open, NULL); free(pctx.arrays), LY_EINVAL); if (!(options & LYD_PRINT_SIBLINGS)) { break; } @@ -1334,6 +1368,7 @@ cbor_print_data(struct ly_out *out, const struct lyd_node *root, uint32_t option if (buffer_size == 0) { ly_set_erase(&pctx.open, NULL); + free(pctx.arrays); return LY_EMEM; } @@ -1343,6 +1378,7 @@ cbor_print_data(struct ly_out *out, const struct lyd_node *root, uint32_t option assert(!pctx.open.count); ly_set_erase(&pctx.open, NULL); + free(pctx.arrays); ly_print_flush(out); return LY_SUCCESS; diff --git a/tests/modules/yang/cbor-test.yang b/tests/modules/yang/cbor-test.yang index af653caa7..09f523763 100644 --- a/tests/modules/yang/cbor-test.yang +++ b/tests/modules/yang/cbor-test.yang @@ -161,6 +161,37 @@ module cbor-test { } } + /* ===== Nested lists / leaf-lists ===== + * Regression coverage for the CBOR printer: a leaf-list and a list + * nested inside another list. Printing such data used to crash because + * the printer kept only a single open-array pointer shared across all + * nesting levels. + */ + + list list-nested { + key "id"; + + leaf id { + type string; + } + + leaf-list tags { + type string; + } + + list inner { + key "iid"; + + leaf iid { + type uint32; + } + + leaf-list vals { + type int32; + } + } + } + /* ===== Choice / Case ===== */ choice choice-transport { diff --git a/tests/utests/data/test_cbor.c b/tests/utests/data/test_cbor.c index d9be9ddc2..e8cd75eae 100644 --- a/tests/utests/data/test_cbor.c +++ b/tests/utests/data/test_cbor.c @@ -129,6 +129,72 @@ test_node(void **state) } } +/** + * @brief Regression test: printing a (leaf-)list nested inside a list. + * + * The CBOR printer used to keep a single "currently open array" pointer in its + * context. When a leaf-list (or list) was nested inside a list, printing the + * inner array overwrote that pointer and then reset it to NULL, so the outer + * list pushed into a NULL array and the printer crashed (SIGSEGV in + * cbor_array_push()). This exercises exactly that structure and additionally + * checks the JSON round-trip fidelity. + */ +static void +test_nested_list(void **state) +{ + struct lyd_node *tree; + char *buffer, *json; + struct ly_out *out; + size_t i; + + const struct test_case tests[] = { + /* leaf-list nested inside a list */ + { + "ll-in-list", + "{\"cbor-test:list-nested\":[" + "{\"id\":\"a\",\"tags\":[\"t1\",\"t2\"]}" + "]}" + }, + /* list nested inside a list, the inner list holding a leaf-list */ + { + "list-in-list", + "{\"cbor-test:list-nested\":[" + "{\"id\":\"a\",\"inner\":[{\"iid\":1,\"vals\":[10,20]},{\"iid\":2,\"vals\":[30]}]}" + "]}" + }, + /* several outer instances mixing nested leaf-lists and lists */ + { + "multi-nested", + "{\"cbor-test:list-nested\":[" + "{\"id\":\"a\",\"tags\":[\"t1\",\"t2\"],\"inner\":[{\"iid\":1,\"vals\":[10,20]}]}," + "{\"id\":\"b\",\"inner\":[{\"iid\":3,\"vals\":[40]}]}" + "]}" + }, + }; + + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); ++i) { + CHECK_PARSE_LYD_PARAM(tests[i].json, LYD_JSON, LYD_PARSE_STRICT, LYD_VALIDATE_PRESENT, LY_SUCCESS, tree); + + assert_int_equal(LY_SUCCESS, ly_out_new_memory(&buffer, 0, &out)); + /* this print call is what used to crash */ + assert_int_equal(LY_SUCCESS, lyd_print_all(out, tree, LYD_CBOR, 0)); + + lyd_free_all(tree); + + assert_int_equal(LY_SUCCESS, lyd_parse_data_mem_len(((struct utest_context *)*state)->ctx, buffer, + (uint32_t)ly_out_printed(out), LYD_CBOR, LYD_PARSE_ONLY, 0, &tree)); + + ly_out_free(out, NULL, 0); + free(buffer); + + assert_int_equal(LY_SUCCESS, lyd_print_mem(&json, tree, LYD_JSON, LYD_PRINT_SHRINK | LYD_PRINT_SIBLINGS)); + assert_string_equal(json, tests[i].json); + + free(json); + lyd_free_all(tree); + } +} + static void test_operations(void **state) { @@ -304,6 +370,7 @@ main(void) { const struct CMUnitTest tests[] = { UTEST(test_node, setup), + UTEST(test_nested_list, setup), UTEST(test_operations, setup), UTEST(test_schema_mount, setup), UTEST(test_opaque, setup),