From 9adffca9c786c3b62eefd9de519287b9801f9fec Mon Sep 17 00:00:00 2001 From: Corey Farrell Date: Mon, 18 Dec 2017 21:12:47 -0500 Subject: CLI: Address multiple issues. * listen uses the variable `s` for the result from ast_poll() then overwrites it with the result of accept(). Create a separate variable poll_result to avoid confusion since ast_poll does not return a file descriptor. * Resolve fd leak that would occur if setsockopt failed in listen. * Reserve an extra byte while processing completion results from remote daemon. This fixes a bug where completion processing used strstr() on a string that was not '\0' terminated. This was no risk to the Asterisk daemon, the bug was only reachable the remote console process. * Resolve leak in handle_showchan when the channel is not found. * Multiple leaks and a deadlock in pbx_config CLI completion. * Fix leaks in "manager show command". Change-Id: I8f633ceb1714867ae30ef4e421858f77c14485a9 --- main/asterisk.c | 34 +++++++++++++++------------------- main/cli.c | 13 ++++++++----- main/manager.c | 27 ++++++++++++++++++++++++++- pbx/pbx_config.c | 14 ++++++++++---- 4 files changed, 59 insertions(+), 29 deletions(-) diff --git a/main/asterisk.c b/main/asterisk.c index 015c9f35a..f8e31d5a0 100644 --- a/main/asterisk.c +++ b/main/asterisk.c @@ -1695,17 +1695,21 @@ static void *listener(void *unused) int s; socklen_t len; int x; + int poll_result; struct pollfd fds[1]; + for (;;) { - if (ast_socket < 0) + if (ast_socket < 0) { return NULL; + } fds[0].fd = ast_socket; fds[0].events = POLLIN; - s = ast_poll(fds, 1, -1); + poll_result = ast_poll(fds, 1, -1); pthread_testcancel(); - if (s < 0) { - if (errno != EINTR) + if (poll_result < 0) { + if (errno != EINTR) { ast_log(LOG_WARNING, "poll returned error: %s\n", strerror(errno)); + } continue; } len = sizeof(sunaddr); @@ -1719,6 +1723,7 @@ static void *listener(void *unused) /* turn on socket credentials passing. */ if (setsockopt(s, SOL_SOCKET, SO_PASSCRED, &sckopt, sizeof(sckopt)) < 0) { ast_log(LOG_WARNING, "Unable to turn on socket credentials passing\n"); + close(s); } else #endif { @@ -3195,20 +3200,10 @@ static char *cli_complete(EditLine *editline, int ch) #define CMD_MATCHESARRAY "_COMMAND MATCHESARRAY \"%s\" \"%s\"" char *mbuf; char *new_mbuf; - int mlen = 0, maxmbuf = 2048; - - /* Start with a 2048 byte buffer */ - mbuf = ast_malloc(maxmbuf); - - /* This will run snprintf twice at most. */ - while (mbuf && (mlen = snprintf(mbuf, maxmbuf, CMD_MATCHESARRAY, lf->buffer, ptr)) > maxmbuf) { - /* Return value does not include space for NULL terminator. */ - maxmbuf = mlen + 1; - ast_free(mbuf); - mbuf = ast_malloc(maxmbuf); - } + int mlen = 0; + int maxmbuf = ast_asprintf(&mbuf, CMD_MATCHESARRAY, lf->buffer, ptr); - if (!mbuf) { + if (maxmbuf == -1) { *((char *) lf->cursor) = savechr; return (char *)(CC_ERROR); @@ -3221,9 +3216,9 @@ static char *cli_complete(EditLine *editline, int ch) while (!strstr(mbuf, AST_CLI_COMPLETE_EOF) && res != -1) { if (mlen + 1024 > maxmbuf) { - /* Expand buffer to the next 1024 byte increment. */ + /* Expand buffer to the next 1024 byte increment plus a NULL terminator. */ maxmbuf = mlen + 1024; - new_mbuf = ast_realloc(mbuf, maxmbuf); + new_mbuf = ast_realloc(mbuf, maxmbuf + 1); if (!new_mbuf) { ast_free(mbuf); *((char *) lf->cursor) = savechr; @@ -3236,6 +3231,7 @@ static char *cli_complete(EditLine *editline, int ch) res = read(ast_consock, mbuf + mlen, 1024); if (res > 0) { mlen += res; + mbuf[mlen] = '\0'; } } mbuf[mlen] = '\0'; diff --git a/main/cli.c b/main/cli.c index 7039b72fc..fe20c3401 100644 --- a/main/cli.c +++ b/main/cli.c @@ -1522,17 +1522,20 @@ static char *handle_showchan(struct ast_cli_entry *e, int cmd, struct ast_cli_ar return CLI_FAILURE; } - output = ast_str_create(8192); - if (!output) { - return CLI_FAILURE; - } - chan = ast_channel_get_by_name(a->argv[3]); if (!chan) { ast_cli(a->fd, "%s is not a known channel\n", a->argv[3]); + return CLI_SUCCESS; } + output = ast_str_create(8192); + if (!output) { + ast_channel_unref(chan); + + return CLI_FAILURE; + } + now = ast_tvnow(); ast_channel_lock(chan); diff --git a/main/manager.c b/main/manager.c index 81052651e..890a97548 100644 --- a/main/manager.c +++ b/main/manager.c @@ -2344,11 +2344,12 @@ static char *handle_showmancmd(struct ast_cli_entry *e, int cmd, struct ast_cli_ AST_RWLIST_UNLOCK(&actions); return ret; } - authority = ast_str_alloca(MAX_AUTH_PERM_STRING); if (a->argc < 4) { return CLI_SHOWUSAGE; } + authority = ast_str_alloca(MAX_AUTH_PERM_STRING); + #ifdef AST_XML_DOCS /* setup the titles */ term_color(synopsis_title, "[Synopsis]\n", COLOR_MAGENTA, 0, 40); @@ -2376,6 +2377,22 @@ static char *handle_showmancmd(struct ast_cli_entry *e, int cmd, struct ast_cli_ char *seealso = ast_xmldoc_printable(S_OR(cur->seealso, "Not available"), 1); char *privilege = ast_xmldoc_printable(S_OR(auth_str, "Not available"), 1); char *responses = ast_xmldoc_printable("None", 1); + + if (!syntax || !synopsis || !description || !arguments + || !seealso || !privilege || !responses) { + ast_free(syntax); + ast_free(synopsis); + ast_free(description); + ast_free(arguments); + ast_free(seealso); + ast_free(privilege); + ast_free(responses); + ast_cli(a->fd, "Allocation failure.\n"); + AST_RWLIST_UNLOCK(&actions); + + return CLI_FAILURE; + } + ast_cli(a->fd, "%s%s\n\n%s%s\n\n%s%s\n\n%s%s\n\n%s%s\n\n%s%s\n\n%s", syntax_title, syntax, synopsis_title, synopsis, @@ -2403,6 +2420,14 @@ static char *handle_showmancmd(struct ast_cli_entry *e, int cmd, struct ast_cli_ ast_cli(a->fd, "Event: %s\n", cur->final_response->name); print_event_instance(a, cur->final_response); } + + ast_free(syntax); + ast_free(synopsis); + ast_free(description); + ast_free(arguments); + ast_free(seealso); + ast_free(privilege); + ast_free(responses); } else #endif { diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c index c4a0e6c28..1a1c73c64 100644 --- a/pbx/pbx_config.c +++ b/pbx/pbx_config.c @@ -303,8 +303,10 @@ static char *complete_dialplan_remove_include(struct ast_cli_args *a) while ( (nc = ast_walk_contexts(nc)) && nc != c && !already_served) already_served = lookup_ci(nc, i_name); - if (!already_served && ++which > a->n) + if (!already_served && ++which > a->n) { res = strdup(i_name); + break; + } } ast_unlock_context(c); } @@ -1523,17 +1525,21 @@ static char *complete_dialplan_remove_ignorepat(struct ast_cli_args *a) } for (c = NULL; !ret && (c = ast_walk_contexts(c)); ) { - if (ast_rdlock_context(c)) /* fail, skip it */ + if (ast_rdlock_context(c)) { + /* fail, skip it */ continue; - if (!partial_match(ast_get_context_name(c), a->word, len)) + } + if (!partial_match(ast_get_context_name(c), a->word, len)) { + ast_unlock_context(c); continue; + } if (lookup_c_ip(c, ignorepat) && ++which > a->n) ret = strdup(ast_get_context_name(c)); ast_unlock_context(c); } ast_unlock_contexts(); free(dupline); - return NULL; + return ret; } return NULL; -- cgit v1.2.3