diff options
author | Scott Griepentrog <sgriepentrog@digium.com> | 2013-12-16 16:12:44 +0000 |
---|---|---|
committer | Scott Griepentrog <sgriepentrog@digium.com> | 2013-12-16 16:12:44 +0000 |
commit | 102d4484868c5b2410c7caf5b99084234c73caf8 (patch) | |
tree | 6d97e534d21797dcd94f76ac2a1ce6859eb4589b | |
parent | 3322180d4b452e11545b70abc9b2d5af3d241361 (diff) |
pbx.c: put copy of ast_exten.data on stack to prevent memory corruption
During dialplan execution in pbx_extension_helper(), the contexts global
read lock prevents link list corruption, but was released with a pointer
to the ast_exten and data later used in variable substitution. Instead,
this patch removes pbx_substitute_variables() and locates a copy of the
ast_exten data on the stack before releasing the lock, where ast_exten
could get free'd by another thread performing a module reload.
(issue AST-1179)
Reported by: Thomas Arimont
(issue AST-1246)
Reported by: Alexander Hömig
Review: https://reviewboard.asterisk.org/r/3055/
........
Merged revisions 403862 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........
Merged revisions 403863 from http://svn.asterisk.org/svn/asterisk/branches/11
........
Merged revisions 403864 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@403865 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r-- | main/pbx.c | 36 |
1 files changed, 16 insertions, 20 deletions
diff --git a/main/pbx.c b/main/pbx.c index 99c686371..ee472d842 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -4627,25 +4627,6 @@ void pbx_substitute_variables_varshead(struct varshead *headp, const char *cp1, pbx_substitute_variables_helper_full(NULL, headp, cp1, cp2, count, &used); } -static void pbx_substitute_variables(char *passdata, int datalen, struct ast_channel *c, struct ast_exten *e) -{ - const char *tmp; - - /* Nothing more to do */ - if (!e->data) { - *passdata = '\0'; - return; - } - - /* No variables or expressions in e->data, so why scan it? */ - if ((!(tmp = strchr(e->data, '$'))) || (!strstr(tmp, "${") && !strstr(tmp, "$["))) { - ast_copy_string(passdata, e->data, datalen); - return; - } - - pbx_substitute_variables_helper(c, e->data, passdata, datalen - 1); -} - /*! * \brief The return value depends on the action: * @@ -4670,6 +4651,7 @@ static int pbx_extension_helper(struct ast_channel *c, struct ast_context *con, { struct ast_exten *e; struct ast_app *app; + char *substitute = NULL; int res; struct pbx_find_info q = { .stacklen = 0 }; /* the rest is reset in pbx_find_extension */ char passdata[EXT_DATA_SIZE]; @@ -4696,6 +4678,18 @@ static int pbx_extension_helper(struct ast_channel *c, struct ast_context *con, if (!e->cached_app) e->cached_app = pbx_findapp(e->app); app = e->cached_app; + if (ast_strlen_zero(e->data)) { + *passdata = '\0'; + } else { + const char *tmp; + if ((!(tmp = strchr(e->data, '$'))) || (!strstr(tmp, "${") && !strstr(tmp, "$["))) { + /* no variables to substitute, copy on through */ + ast_copy_string(passdata, e->data, sizeof(passdata)); + } else { + /* save e->data on stack for later processing after lock released */ + substitute = ast_strdupa(e->data); + } + } ast_unlock_contexts(); if (!app) { ast_log(LOG_WARNING, "No application '%s' for extension (%s, %s, %d)\n", e->app, context, exten, priority); @@ -4706,7 +4700,9 @@ static int pbx_extension_helper(struct ast_channel *c, struct ast_context *con, if (ast_channel_exten(c) != exten) ast_channel_exten_set(c, exten); ast_channel_priority_set(c, priority); - pbx_substitute_variables(passdata, sizeof(passdata), c, e); + if (substitute) { + pbx_substitute_variables_helper(c, substitute, passdata, sizeof(passdata)-1); + } ast_debug(1, "Launching '%s'\n", app->name); { ast_verb(3, "Executing [%s@%s:%d] " COLORIZE_FMT "(\"" COLORIZE_FMT "\", \"" COLORIZE_FMT "\") %s\n", |