From 9b189998e0a055230dcfcd359015b98453aa85cc Mon Sep 17 00:00:00 2001 From: aravindor Date: Sat, 7 Nov 2026 16:02:57 +0530 Subject: [PATCH] data REFACTOR build strings with measured, bounded appends The XPath string cast, concat() and the JSON predicate, lyd_path(), the LYB feature check and the CBOR number conversion grew strings by summing each length by hand and writing with sprintf()/strcpy()/strcat(). The sums are correct today but each one has to keep matching its format string, and these paths format values that come from parsed data. Measure the formatted output with vsnprintf() and write it bounded instead, as #2591 did for the type plugins. Output is unchanged. - xpath.c: new cast_string_append() keeps the step growth of cast_string_realloc(); concat() and the predicate builder use ly_append_str(). - tree_data.c: new lyd_path_str_append() on top of lyd_path_str_enlarge(), so static buffers still return LY_EINCOMPLETE. - parser_lyb.c: feature lists use ly_append_str(); the revision string uses snprintf(). This also fixes lyb_check_mod_features() returning LY_SUCCESS when allocating the error message failed, which accepted mismatched features. - parser_cbor.c, parser_common.c: snprintf() bounded by the allocation, strdup() instead of malloc()+strcpy(). --- src/parser_cbor.c | 6 +-- src/parser_common.c | 5 +-- src/parser_lyb.c | 43 +++++-------------- src/tree_data.c | 97 +++++++++++++++++++++++------------------- src/xpath.c | 101 ++++++++++++++++++++++++++------------------ 5 files changed, 128 insertions(+), 124 deletions(-) diff --git a/src/parser_cbor.c b/src/parser_cbor.c index 1d3e1dd7d..2d28058bc 100644 --- a/src/parser_cbor.c +++ b/src/parser_cbor.c @@ -446,7 +446,7 @@ lydcbor_item_to_string(const struct ly_ctx *ctx, const cbor_item_t *item, char * } *str_val = malloc(len + 1); LY_CHECK_ERR_RET(!*str_val, LOGMEM(ctx), LY_EMEM); - sprintf(*str_val, "%" PRIu64, val); + snprintf(*str_val, len + 1, "%" PRIu64, val); *str_len = len; break; } @@ -468,7 +468,7 @@ lydcbor_item_to_string(const struct ly_ctx *ctx, const cbor_item_t *item, char * } *str_val = malloc(len + 1); LY_CHECK_ERR_RET(!*str_val, LOGMEM(ctx), LY_EMEM); - sprintf(*str_val, "-%" PRIu64, val + 1); + snprintf(*str_val, len + 1, "-%" PRIu64, val + 1); *str_len = len; break; } @@ -526,7 +526,7 @@ lydcbor_item_to_string(const struct ly_ctx *ctx, const cbor_item_t *item, char * } *str_val = malloc(len + 1); LY_CHECK_ERR_RET(!*str_val, LOGMEM(ctx), LY_EMEM); - sprintf(*str_val, "%g", val); + snprintf(*str_val, len + 1, "%g", val); *str_len = len; } break; diff --git a/src/parser_common.c b/src/parser_common.c index 5d4961b96..e4214ce87 100644 --- a/src/parser_common.c +++ b/src/parser_common.c @@ -463,7 +463,7 @@ lys_parser_fill_filepath(struct ly_ctx *ctx, struct ly_in *in, const char **file } #else /* get URI if there is /proc */ - sprintf(proc_path, "/proc/self/fd/%d", in->method.fd); + snprintf(proc_path, sizeof proc_path, "/proc/self/fd/%d", in->method.fd); if ((len = readlink(proc_path, path, PATH_MAX - 1)) > 0) { LY_CHECK_RET(lysdict_insert(ctx, path, len, filepath)); } @@ -1339,9 +1339,8 @@ lysp_stmt_type_pattern_modifier(struct lysp_ctx *ctx, const struct lysp_stmt *st } /* replace the value in the dictionary */ - buf = malloc(strlen(*pat) + 1); + buf = strdup(*pat); LY_CHECK_ERR_RET(!buf, LOGMEM(PARSER_CTX(ctx)), LY_EMEM); - strcpy(buf, *pat); lysdict_remove(PARSER_CTX(ctx), *pat); assert(buf[0] == LYSP_RESTR_PATTERN_ACK); diff --git a/src/parser_lyb.c b/src/parser_lyb.c index 5065e2182..95691b9b8 100644 --- a/src/parser_lyb.c +++ b/src/parser_lyb.c @@ -422,7 +422,7 @@ lyb_check_mod_features(struct lyd_lyb_ctx *lybctx, const char *mod_name, const s LY_ERR rc = LY_SUCCESS; struct lylyb_parse_ctx *pctx = lybctx->parse_ctx; char *feature_name = NULL, **features = NULL, *en_feats_printer = NULL, *en_feats_parser = NULL; - uint32_t i, feature_count = 0, ctx_feature_count, len; + uint32_t i, feature_count = 0, ctx_feature_count, size = 0, used = 0; /* read feature count */ LY_CHECK_GOTO(rc = lyb_read_count(&feature_count, pctx), cleanup); @@ -461,47 +461,23 @@ lyb_check_mod_features(struct lyd_lyb_ctx *lybctx, const char *mod_name, const s } /* feature mismatch, prepare error message, unpack enabled features for printer */ - len = 0; for (i = 0; i < feature_count; i++) { - if ((uint64_t)len + strlen(features[i]) + 2 > UINT32_MAX - 1) { - LOGERR(pctx->ctx, LY_EINVAL, "Invalid enabled features of module \"%s\", too long.", mod_name); - rc = LY_EDENIED; - goto cleanup; - } - len += strlen(features[i]) + 2; - } - en_feats_printer = calloc(1, len + 1); - LY_CHECK_ERR_GOTO(!en_feats_printer, LOGMEM(pctx->ctx), cleanup); - for (i = 0; i < feature_count; i++) { - strcat(en_feats_printer, features[i]); - if (i + 1 < feature_count) { - strcat(en_feats_printer, ", "); - } + LY_CHECK_GOTO(rc = ly_append_str(&en_feats_printer, &size, &used, "%s%s", i ? ", " : "", features[i]), + cleanup); } /* unpack enabled features for parser */ - len = 0; + size = 0; + used = 0; ctx_feature_count = LYA_COUNT(mod->compiled->features); for (i = 0; i < ctx_feature_count; i++) { - if ((uint64_t)len + strlen(mod->compiled->features[i]) + 2 > UINT32_MAX - 1) { - LOGINT(pctx->ctx); - rc = LY_EINT; - goto cleanup; - } - len += strlen(mod->compiled->features[i]) + 2; - } - en_feats_parser = calloc(1, len + 1); - LY_CHECK_ERR_GOTO(!en_feats_parser, LOGMEM(pctx->ctx), cleanup); - for (i = 0; i < ctx_feature_count; i++) { - strcat(en_feats_parser, mod->compiled->features[i]); - if (i + 1 < ctx_feature_count) { - strcat(en_feats_parser, ", "); - } + LY_CHECK_GOTO(rc = ly_append_str(&en_feats_parser, &size, &used, "%s%s", i ? ", " : "", + mod->compiled->features[i]), cleanup); } LOGERR(pctx->ctx, LY_EDENIED, "Invalid context for LYB data parsing, module \"%s\" enabled feature mismatch (printer: {%s}, parser: {%s}).", - mod_name, en_feats_printer, en_feats_parser); + mod_name, en_feats_printer ? en_feats_printer : "", en_feats_parser ? en_feats_parser : ""); rc = LY_EDENIED; cleanup: @@ -546,7 +522,8 @@ lyb_parse_module(struct lyd_lyb_ctx *lybctx, const struct lys_module **mod) /* module revision */ LY_CHECK_GOTO(rc = lyb_read(&rev, 2 * 8, pctx), cleanup); if (rev) { - sprintf(mod_rev, "%04u-%02u-%02u", ((rev & LYB_REV_YEAR_MASK) >> LYB_REV_YEAR_SHIFT) + LYB_REV_YEAR_OFFSET, + snprintf(mod_rev, sizeof mod_rev, "%04u-%02u-%02u", + ((rev & LYB_REV_YEAR_MASK) >> LYB_REV_YEAR_SHIFT) + LYB_REV_YEAR_OFFSET, (rev & LYB_REV_MONTH_MASK) >> LYB_REV_MONTH_SHIFT, rev & LYB_REV_DAY_MASK); } diff --git a/src/tree_data.c b/src/tree_data.c index 56eb01d4d..d0bb5b729 100644 --- a/src/tree_data.c +++ b/src/tree_data.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -2944,21 +2945,58 @@ lyd_path_str_enlarge(char **buffer, size_t *buflen, size_t reqlen, ly_bool is_st return LY_SUCCESS; } +static LY_ERR lyd_path_str_append(char **buffer, size_t *buflen, size_t *bufused, ly_bool is_static, + const char *format, ...) _FORMAT_PRINTF(5, 6); + +/** + * @brief Append formatted text to a path, enlarging the buffer if it is not static. + * + * The length of the appended text is measured from the format itself so that callers never compute it by hand. + * + * @param[in,out] buffer Buffer to print to. + * @param[in,out] buflen Current buffer length. + * @param[in,out] bufused Current number of characters used in @p buffer. + * @param[in] is_static Whether buffer is static or can be reallocated. + * @param[in] format Format of the appended text. + * @param[in] ... Format arguments. + * @return LY_ERR value, LY_EINCOMPLETE if a static @p buffer is too small. + */ +static LY_ERR +lyd_path_str_append(char **buffer, size_t *buflen, size_t *bufused, ly_bool is_static, const char *format, ...) +{ + va_list ap; + int len; + + va_start(ap, format); + len = vsnprintf(NULL, 0, format, ap); + va_end(ap); + if (len < 0) { + LOGERR(NULL, LY_ESYS, "Failed to print a path (%s).", strerror(errno)); + return LY_ESYS; + } + + LY_CHECK_RET(lyd_path_str_enlarge(buffer, buflen, *bufused + len, is_static)); + + va_start(ap, format); + vsnprintf(*buffer + *bufused, *buflen - *bufused, format, ap); + va_end(ap); + *bufused += len; + + return LY_SUCCESS; +} + LY_ERR lyd_path_list_predicate(const struct lyd_node *node, char **buffer, size_t *buflen, size_t *bufused, ly_bool is_static) { const struct lyd_node *key; - size_t len; const char *val; char quot; for (key = lyd_child(node); key && key->schema && (key->schema->flags & LYS_KEY); key = key->next) { val = lyd_get_value(key); - len = 1 + strlen(key->schema->name) + 2 + strlen(val) + 2; - LY_CHECK_RET(lyd_path_str_enlarge(buffer, buflen, *bufused + len, is_static)); - LY_CHECK_RET(ly_val_get_quot(LYD_CTX(node), val, ")); - *bufused += sprintf(*buffer + *bufused, "[%s=%c%s%c]", key->schema->name, quot, val, quot); + LY_CHECK_RET(lyd_path_str_append(buffer, buflen, bufused, is_static, "[%s=%c%s%c]", key->schema->name, quot, + val, quot)); } return LY_SUCCESS; @@ -2977,16 +3015,12 @@ lyd_path_list_predicate(const struct lyd_node *node, char **buffer, size_t *bufl static LY_ERR lyd_path_leaflist_predicate(const struct lyd_node *node, char **buffer, size_t *buflen, size_t *bufused, ly_bool is_static) { - size_t len; const char *val; char quot; val = lyd_get_value(node); - len = 4 + strlen(val) + 2; /* "[.='" + val + "']" */ - LY_CHECK_RET(lyd_path_str_enlarge(buffer, buflen, *bufused + len, is_static)); - LY_CHECK_RET(ly_val_get_quot(LYD_CTX(node), val, ")); - *bufused += sprintf(*buffer + *bufused, "[.=%c%s%c]", quot, val, quot); + LY_CHECK_RET(lyd_path_str_append(buffer, buflen, bufused, is_static, "[.=%c%s%c]", quot, val, quot)); return LY_SUCCESS; } @@ -3004,27 +3038,10 @@ lyd_path_leaflist_predicate(const struct lyd_node *node, char **buffer, size_t * static LY_ERR lyd_path_position_predicate(const struct lyd_node *node, char **buffer, size_t *buflen, size_t *bufused, ly_bool is_static) { - size_t len; uint32_t pos; - char *val = NULL; - LY_ERR rc; pos = lyd_list_pos(node); - if (asprintf(&val, "%" PRIu32, pos) == -1) { - return LY_EMEM; - } - - len = 1 + strlen(val) + 1; - rc = lyd_path_str_enlarge(buffer, buflen, *bufused + len, is_static); - if (rc != LY_SUCCESS) { - goto cleanup; - } - - *bufused += sprintf(*buffer + *bufused, "[%s]", val); - -cleanup: - free(val); - return rc; + return lyd_path_str_append(buffer, buflen, bufused, is_static, "[%" PRIu32 "]", pos); } LIBYANG_API_DEF char * @@ -3032,7 +3049,7 @@ lyd_path(const struct lyd_node *node, LYD_PATH_TYPE pathtype, char *buffer, size { ly_bool is_static = 0; uint32_t i, depth; - size_t bufused = 0, len; + size_t bufused = 0; const struct lyd_node *iter, *parent; const struct lys_module *mod, *prev_mod; LY_ERR rc = LY_SUCCESS; @@ -3066,17 +3083,13 @@ lyd_path(const struct lyd_node *node, LYD_PATH_TYPE pathtype, char *buffer, size mod = NULL; } - /* realloc string */ - len = 1 + (mod ? strlen(mod->name) + 1 : 0) + (iter->schema ? strlen(iter->schema->name) : - strlen(((struct lyd_node_opaq *)iter)->name.name)); - rc = lyd_path_str_enlarge(&buffer, &buflen, bufused + len, is_static); + /* print next node */ + rc = lyd_path_str_append(&buffer, &buflen, &bufused, is_static, "/%s%s%s", mod ? mod->name : "", + mod ? ":" : "", LYD_NAME(iter)); if (rc) { break; } - /* print next node */ - bufused += sprintf(buffer + bufused, "/%s%s%s", mod ? mod->name : "", mod ? ":" : "", LYD_NAME(iter)); - /* do not always print the last (first) predicate */ if (iter->schema && ((depth > 1) || (pathtype == LYD_PATH_STD))) { switch (iter->schema->nodetype) { @@ -3123,7 +3136,7 @@ char * lyd_path_set(const struct ly_set *dnodes, LYD_PATH_TYPE pathtype) { uint32_t depth; - size_t bufused = 0, buflen = 0, len; + size_t bufused = 0, buflen = 0; char *buffer = NULL; const struct lyd_node *iter, *parent; const struct lys_module *mod, *prev_mod; @@ -3149,16 +3162,12 @@ lyd_path_set(const struct ly_set *dnodes, LYD_PATH_TYPE pathtype) mod = NULL; } - /* realloc string */ - len = 1 + (mod ? strlen(mod->name) + 1 : 0) + (iter->schema ? strlen(iter->schema->name) : - strlen(((struct lyd_node_opaq *)iter)->name.name)); - if ((rc = lyd_path_str_enlarge(&buffer, &buflen, bufused + len, 0))) { + /* print next node */ + if ((rc = lyd_path_str_append(&buffer, &buflen, &bufused, 0, "/%s%s%s", mod ? mod->name : "", + mod ? ":" : "", LYD_NAME(iter)))) { break; } - /* print next node */ - bufused += sprintf(buffer + bufused, "/%s%s%s", mod ? mod->name : "", mod ? ":" : "", LYD_NAME(iter)); - /* do not always print the last (first) predicate */ if (iter->schema && ((depth > 1) || (pathtype == LYD_PATH_STD))) { switch (iter->schema->nodetype) { diff --git a/src/xpath.c b/src/xpath.c index d0bbdf7f0..82ef5f376 100644 --- a/src/xpath.c +++ b/src/xpath.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -426,6 +427,47 @@ cast_string_realloc(const struct ly_ctx *ctx, uint64_t needed, char **str, uint3 return LY_SUCCESS; } +static LY_ERR cast_string_append(const struct ly_ctx *ctx, char **str, uint32_t *used, uint32_t *size, + const char *format, ...) _FORMAT_PRINTF(5, 6); + +/** + * @brief Append formatted text to a string being cast, enlarging it as needed. + * + * The length of the appended text is measured from the format itself so that callers never compute it by hand. + * + * @param[in] ctx libyang context for logging. + * @param[in,out] str Pointer to the string to use. + * @param[in,out] used Used bytes in \p str, including the terminating zero. + * @param[in,out] size Allocated bytes in \p str. + * @param[in] format Format of the appended text. + * @param[in] ... Format arguments. + * @return LY_ERR + */ +static LY_ERR +cast_string_append(const struct ly_ctx *ctx, char **str, uint32_t *used, uint32_t *size, const char *format, ...) +{ + va_list ap; + int len; + + va_start(ap, format); + len = vsnprintf(NULL, 0, format, ap); + va_end(ap); + if (len < 0) { + LOGERR(ctx, LY_ESYS, "Failed to print a string (%s).", strerror(errno)); + return LY_ESYS; + } + + /* the terminating zero is overwritten, so len more bytes are needed */ + LY_CHECK_RET(cast_string_realloc(ctx, len, str, used, size)); + + va_start(ap, format); + vsnprintf(*str + (*used - 1), *size - (*used - 1), format, ap); + va_end(ap); + *used += len; + + return LY_SUCCESS; +} + /** * @brief Cast nodes recursively to one string @p str. * @@ -455,9 +497,7 @@ cast_string_recursive(const struct lyd_node *node, struct lyxp_set *set, uint32_ if (!node) { /* fake container */ - LY_CHECK_RET(cast_string_realloc(set->ctx, 1, str, used, size)); - strcpy(*str + (*used - 1), "\n"); - ++(*used); + LY_CHECK_RET(cast_string_append(set->ctx, str, used, size, "\n")); ++indent; @@ -469,9 +509,7 @@ cast_string_recursive(const struct lyd_node *node, struct lyxp_set *set, uint32_ } /* end fake container */ - LY_CHECK_RET(cast_string_realloc(set->ctx, 1, str, used, size)); - strcpy(*str + (*used - 1), "\n"); - ++(*used); + LY_CHECK_RET(cast_string_append(set->ctx, str, used, size, "\n")); --indent; } else { @@ -488,9 +526,7 @@ cast_string_recursive(const struct lyd_node *node, struct lyxp_set *set, uint32_ case LYS_LIST: case LYS_RPC: case LYS_NOTIF: - LY_CHECK_RET(cast_string_realloc(set->ctx, 1, str, used, size)); - strcpy(*str + (*used - 1), "\n"); - ++(*used); + LY_CHECK_RET(cast_string_append(set->ctx, str, used, size, "\n")); for (child = lyd_child(node); child; child = child->next) { LY_CHECK_RET(cast_string_recursive(child, set, indent + 1, str, used, size)); @@ -502,19 +538,9 @@ cast_string_recursive(const struct lyd_node *node, struct lyxp_set *set, uint32_ case LYS_LEAFLIST: value_str = lyd_get_value(node); - /* print indent */ - LY_CHECK_RET(cast_string_realloc(set->ctx, indent * 2 + strlen(value_str) + 1, str, used, size)); - memset(*str + (*used - 1), ' ', indent * 2); - *used += indent * 2; - - /* print value */ - if (*used == 1) { - sprintf(*str + (*used - 1), "%s", value_str); - *used += strlen(value_str); - } else { - sprintf(*str + (*used - 1), "%s\n", value_str); - *used += strlen(value_str) + 1; - } + /* print indent and value, a lone top-level value has no trailing newline */ + LY_CHECK_RET(cast_string_append(set->ctx, str, used, size, "%*s%s%s", (int)(indent * 2), "", value_str, + ((*used == 1) && !indent) ? "" : "\n")); break; @@ -540,19 +566,11 @@ cast_string_recursive(const struct lyd_node *node, struct lyxp_set *set, uint32_ } for (line = strtok_r(buf, "\n", &ptr); line; line = strtok_r(NULL, "\n", &ptr)) { - rc = cast_string_realloc(set->ctx, indent * 2 + strlen(line) + 1, str, used, size); + rc = cast_string_append(set->ctx, str, used, size, "%*s%s\n", (int)(indent * 2), "", line); if (rc != LY_SUCCESS) { free(buf); return rc; } - memset(*str + (*used - 1), ' ', indent * 2); - *used += indent * 2; - - strcpy(*str + (*used - 1), line); - *used += strlen(line); - - strcpy(*str + (*used - 1), "\n"); - *used += 1; } free(buf); @@ -3839,7 +3857,7 @@ xpath_concat(struct lyxp_set **args, uint32_t arg_count, struct lyxp_set *set, u { uint32_t i; char *str = NULL; - size_t used = 1; + uint32_t size = 0, used = 0; LY_ERR rc = LY_SUCCESS; struct lysc_node_leaf *sleaf; @@ -3868,10 +3886,11 @@ xpath_concat(struct lyxp_set **args, uint32_t arg_count, struct lyxp_set *set, u return rc; } - str = ly_realloc(str, (used + strlen(args[i]->val.str)) * sizeof(char)); - LY_CHECK_ERR_RET(!str, LOGMEM(set->ctx), LY_EMEM); - strcpy(str + used - 1, args[i]->val.str); - used += strlen(args[i]->val.str); + rc = ly_append_str(&str, &size, &used, "%s", args[i]->val.str); + if (rc != LY_SUCCESS) { + free(str); + return rc; + } } /* free, kind of */ @@ -7953,7 +7972,7 @@ eval_name_test_try_compile_predicate_append(const struct lyxp_expr *exp, uint32_ uint32_t *pred_len) { LY_ERR rc = LY_SUCCESS; - uint32_t i; + uint32_t i, pred_size; const struct lyd_node *siblings; struct lyd_node *ctx_node; const struct lysc_node *sparent, *cur_scnode; @@ -8012,11 +8031,11 @@ eval_name_test_try_compile_predicate_append(const struct lyxp_expr *exp, uint32_ /* cast it into a string */ LY_CHECK_GOTO(rc = lyxp_set_cast(&set2, LYXP_SET_STRING), cleanup); - /* append the JSON predicate */ - *pred = ly_realloc(*pred, *pred_len + 1 + strlen(pred_node->name) + 2 + strlen(set2.val.str) + 3); - LY_CHECK_ERR_GOTO(!*pred, LOGMEM(set->ctx); rc = LY_EMEM, cleanup); + /* append the JSON predicate, *pred is allocated at least for *pred_len characters and the terminating zero */ LY_CHECK_GOTO(rc = ly_val_get_quot(set->ctx, set2.val.str, "), cleanup); - *pred_len += sprintf(*pred + *pred_len, "[%s=%c%s%c]", pred_node->name, quot, set2.val.str, quot); + pred_size = *pred ? *pred_len + 1 : 0; + LY_CHECK_GOTO(rc = ly_append_str(pred, &pred_size, pred_len, "[%s=%c%s%c]", pred_node->name, quot, + set2.val.str, quot), cleanup); cleanup: lyxp_expr_free(val_exp);