From 650a93212fb6f8c8132293df99f61eed56ecf205 Mon Sep 17 00:00:00 2001 From: aravindor Date: Mon, 9 Nov 2026 21:34:40 +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 has to keep matching its format string, and these paths format values that come from parsed data. Measure the formatted output and write it bounded instead, as #2591 did for the type plugins. Output is unchanged. - xpath.c: new cast_string_append() measures with vsnprintf() and keeps the step growth of the former cast_string_realloc(), now inlined into it. concat() and the predicate builder use ly_append_str(). - tree_data.c: new lyd_path_str_append() with the former lyd_path_str_enlarge() inlined, so a static buffer still returns 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 | 85 +++++++++++++++++----------------------- src/xpath.c | 95 ++++++++++++++++++++++----------------------- 5 files changed, 97 insertions(+), 137 deletions(-) diff --git a/src/parser_cbor.c b/src/parser_cbor.c index ba810dcd5..44eafa6b2 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 100398305..c341be5db 100644 --- a/src/parser_common.c +++ b/src/parser_common.c @@ -479,7 +479,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)); } @@ -1355,9 +1355,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 5f75f795d..c1e813869 100644 --- a/src/tree_data.c +++ b/src/tree_data.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -54,6 +55,8 @@ #include "xpath.h" static int lyd_insert_has_keys(const struct lyd_node *list); +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); static LY_ERR lyd_compare_siblings_(const struct lyd_node *node1, const struct lyd_node *node2, uint32_t options, ly_bool parental_schemas_checked); @@ -2936,24 +2939,38 @@ lyd_merge_module(struct lyd_node **target, const struct lyd_node *source, const } static LY_ERR -lyd_path_str_enlarge(char **buffer, size_t *buflen, size_t reqlen, ly_bool is_static) +lyd_path_str_append(char **buffer, size_t *buflen, size_t *bufused, ly_bool is_static, const char *format, ...) { - /* ending \0 */ - ++reqlen; + va_list ap; + int len; - if (reqlen > *buflen) { + 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; + } + + /* grow the buffer if it is not static, +1 for the terminating zero */ + if (*bufused + len + 1 > *buflen) { if (is_static) { return LY_EINCOMPLETE; } - *buffer = ly_realloc(*buffer, reqlen * sizeof **buffer); + *buffer = ly_realloc(*buffer, (*bufused + len + 1) * sizeof **buffer); if (!*buffer) { return LY_EMEM; } - *buflen = reqlen; + *buflen = *bufused + len + 1; } + va_start(ap, format); + vsnprintf(*buffer + *bufused, *buflen - *bufused, format, ap); + va_end(ap); + *bufused += len; + return LY_SUCCESS; } @@ -2961,17 +2978,14 @@ 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; @@ -2990,16 +3004,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; } @@ -3017,27 +3027,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 * @@ -3045,7 +3038,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; @@ -3079,17 +3072,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) { @@ -3136,7 +3125,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; @@ -3162,16 +3151,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 a97c3fc9a..c8af3d395 100644 --- a/src/xpath.c +++ b/src/xpath.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -56,6 +57,8 @@ static LY_ERR moveto_node(struct lyxp_set *set, const char *ncname, uint32_t ncn static LY_ERR moveto_scnode(struct lyxp_set *set, const char *ncname, uint32_t ncname_len, enum lyxp_axis axis, uint32_t options); static LY_ERR moveto_op_comp(struct lyxp_set *set1, struct lyxp_set *set2, const char *op, ly_bool *result); +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); #define LOGVAL_DXPATH(SET, ...) \ LOGVAL((SET)->ctx, (SET)->cur_node, __VA_ARGS__); @@ -397,30 +400,50 @@ print_set_debug(struct lyxp_set *set) #endif /** - * @brief Realloc the string \p str. + * @brief Append formatted text to a string being cast, enlarging it as needed. + * + * The length of the appended text is measured from the format, so callers never size the buffer by hand. * * @param[in] ctx libyang context for logging. - * @param[in] needed How much free space is required. * @param[in,out] str Pointer to the string to use. - * @param[in,out] used Used bytes in \p str. + * @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_realloc(const struct ly_ctx *ctx, uint64_t needed, char **str, uint32_t *used, uint32_t *size) +cast_string_append(const struct ly_ctx *ctx, char **str, uint32_t *used, uint32_t *size, const char *format, ...) { - if (*size - (unsigned)*used < needed) { + 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 */ + if (*size - *used < (unsigned)len) { do { if ((UINT32_MAX - *size) < LYXP_STRING_CAST_SIZE_STEP) { LOGERR(ctx, LY_EINVAL, "XPath string length limit (%" PRIu32 ") reached.", UINT32_MAX); return LY_EINVAL; } *size += LYXP_STRING_CAST_SIZE_STEP; - } while (*size - (unsigned)*used < needed); + } while (*size - *used < (unsigned)len); *str = ly_realloc(*str, *size * sizeof(char)); LY_CHECK_ERR_RET(!(*str), LOGMEM(ctx), LY_EMEM); } + va_start(ap, format); + vsnprintf(*str + (*used - 1), *size - (*used - 1), format, ap); + va_end(ap); + *used += len; + return LY_SUCCESS; } @@ -453,9 +476,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; @@ -467,9 +488,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 { @@ -486,9 +505,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)); @@ -500,19 +517,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; @@ -538,19 +545,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); @@ -4017,9 +4016,8 @@ xpath_ceiling(struct lyxp_set **args, uint32_t UNUSED(arg_count), struct lyxp_se static LY_ERR xpath_concat(struct lyxp_set **args, uint32_t arg_count, struct lyxp_set *set, uint32_t options) { - uint32_t i; + uint32_t i, size = 0, used = 0; char *str = NULL; - size_t used = 1; LY_ERR rc = LY_SUCCESS; struct lysc_node_leaf *sleaf; @@ -4048,10 +4046,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 */ @@ -6504,7 +6503,7 @@ moveto_node_try_compile_predicate_append(const struct lyxp_expr *exp, uint32_t t 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; @@ -6563,11 +6562,11 @@ moveto_node_try_compile_predicate_append(const struct lyxp_expr *exp, uint32_t t /* 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 holds at least *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);