From ca26d3b93e673e419e5c36a3dd41b214e049a9b5 Mon Sep 17 00:00:00 2001 From: Mark Murawski Date: Sun, 23 Mar 2025 20:22:41 -0400 Subject: [PATCH] Unsquashed commit -- Changes as requested: - Rebase master (Now all unit tests pass) - Fix seanbright review issue: ast_var_set problems Resolution: Turns out that it's not so simple to just overwrite the existing string. Best option is to just remove and add like everyone else Resolves: Memory overwrite when overwriting existing group variable with larger string --- include/asterisk/chanvars.h | 15 --------------- main/app.c | 24 +++++++++--------------- main/chanvars.c | 7 ------- 3 files changed, 9 insertions(+), 37 deletions(-) diff --git a/include/asterisk/chanvars.h b/include/asterisk/chanvars.h index f16c595f09..c867baef9e 100644 --- a/include/asterisk/chanvars.h +++ b/include/asterisk/chanvars.h @@ -63,21 +63,6 @@ struct ast_var_t *_ast_var_assign(const char *name, const char *value, const cha */ #define ast_var_assign(name, value) _ast_var_assign(name, value, __FILE__, __LINE__, __PRETTY_FUNCTION__) -/*! - * \brief Replace the value of an existing variable - * \pre Caller assumes all responsibility for handling locking - * - * \see ast_var_find - * \see ast_var_assign - * - * \param var must be a struct ast_var_t * that's been set up with ast_var_assign - * \param new_value value of the variable to set - * - * \retval 1 - * \retval 0 on success - */ -int ast_var_set(const struct ast_var_t *var, const char *new_value); - /*! * \brief Free a variable. This does not remove the variable from the list that it might be a part of * \see linkedlists.h diff --git a/main/app.c b/main/app.c index 9860302ebe..f2f170d5b5 100644 --- a/main/app.c +++ b/main/app.c @@ -2463,9 +2463,8 @@ int ast_app_group_set_var(struct ast_channel *chan, const char *group, const cha struct ast_group_meta *gmi = NULL; struct varshead *headp; - struct ast_var_t *newvariable = NULL; - int existing_var_found = 0; - + struct ast_var_t *variable = NULL; + if (!group || !name) { ast_log(LOG_WARNING, "<%s> GROUP variable assignment failed for %s@%s, group/name cannot be NULL, group variable '%s' not set\n", ast_channel_name(chan), group, category, name); return -2; @@ -2488,22 +2487,17 @@ int ast_app_group_set_var(struct ast_channel *chan, const char *group, const cha headp = &gmi->varshead; - AST_LIST_TRAVERSE(headp, newvariable, entries) { - if (strcasecmp(ast_var_name(newvariable), name) == 0) { - /* there is already such a variable, replace it */ - existing_var_found = 1; - ast_var_set(newvariable, value); + AST_LIST_TRAVERSE(headp, variable, entries) { + if (strcasecmp(ast_var_name(variable), name) == 0) { + /* there is already such a variable, nuke it so we can replace it */ + ast_var_delete(variable); break; } } - if (existing_var_found) { - break; - } - - newvariable = ast_var_assign(name, value); + variable = ast_var_assign(name, value); - AST_LIST_INSERT_HEAD(headp, newvariable, entries); + AST_LIST_INSERT_HEAD(headp, variable, entries); manager_event(EVENT_FLAG_DIALPLAN, "GroupVarSet", "Channel: %s\r\n" "Category: %s\r\n" @@ -2520,7 +2514,7 @@ int ast_app_group_set_var(struct ast_channel *chan, const char *group, const cha AST_RWLIST_TRAVERSE_SAFE_END; AST_RWLIST_UNLOCK(&groups_meta); - if (!newvariable) { + if (!variable) { ast_log(LOG_WARNING, "<%s> GROUP assignment %s@%s doesn't exist, group variable '%s' not set\n", ast_channel_name(chan), group, category, name); return -1; } diff --git a/main/chanvars.c b/main/chanvars.c index 45e9495640..0802646b90 100644 --- a/main/chanvars.c +++ b/main/chanvars.c @@ -52,13 +52,6 @@ struct ast_var_t *_ast_var_assign(const char *name, const char *value, const cha return var; } -int ast_var_set(const struct ast_var_t *var, const char *new_value) -{ - ast_copy_string(var->value, new_value, strlen(new_value)); - - return 0; -} - void ast_var_delete(struct ast_var_t *var) { ast_free(var);