[Asterisk-code-review] pbx spool: Gracefully handle long lines in call files (asterisk[14])
Anonymous Coward
asteriskteam at digium.com
Tue Mar 7 17:22:49 CST 2017
Anonymous Coward #1000019 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5126 )
Change subject: pbx_spool: Gracefully handle long lines in call files
......................................................................
pbx_spool: Gracefully handle long lines in call files
Per the linked issue, we aren't checking the buffer filled by fgets()
to determine if it contains a newline, so we will fail to correctly
parse the trailing portion of a long line.
This patch increases the buffer size from 256 to 1024, and skips any
line that exceeds that length, logging a warning in the process.
ASTERISK-17067 #close
Reported by: Dave Olszewski
Change-Id: I51bcf270c1b4347ba05b43f18dc2094c76f5d7b0
---
M pbx/pbx_spool.c
1 file changed, 154 insertions(+), 122 deletions(-)
Approvals:
George Joseph: Looks good to me, approved
Richard Mudgett: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
diff --git a/pbx/pbx_spool.c b/pbx/pbx_spool.c
index ae77d9b..a2a74e6 100644
--- a/pbx/pbx_spool.c
+++ b/pbx/pbx_spool.c
@@ -163,139 +163,171 @@
return o;
}
-static int apply_outgoing(struct outgoing *o, FILE *f)
+static void parse_line(char *line, unsigned int lineno, struct outgoing *o)
{
- char buf[256];
- char *c, *c2;
- int lineno = 0;
- struct ast_variable *var, *last = o->vars;
+ char *c;
- while (last && last->next) {
- last = last->next;
+ /* Trim comments */
+ c = line;
+ while ((c = strchr(c, '#'))) {
+ if ((c == line) || (*(c-1) == ' ') || (*(c-1) == '\t')) {
+ *c = '\0';
+ break;
+ }
+ c++;
}
- while(fgets(buf, sizeof(buf), f)) {
+ c = line;
+ while ((c = strchr(c, ';'))) {
+ if ((c > line) && (c[-1] == '\\')) {
+ memmove(c - 1, c, strlen(c) + 1);
+ } else {
+ *c = '\0';
+ break;
+ }
+ }
+
+ /* Trim trailing white space */
+ ast_trim_blanks(line);
+ if (ast_strlen_zero(line)) {
+ return;
+ }
+ c = strchr(line, ':');
+ if (!c) {
+ ast_log(LOG_NOTICE, "Syntax error at line %d of %s\n", lineno, o->fn);
+ return;
+ }
+ *c = '\0';
+ c = ast_skip_blanks(c + 1);
+#if 0
+ printf("'%s' is '%s' at line %d\n", line, c, lineno);
+#endif
+ if (!strcasecmp(line, "channel")) {
+ char *c2;
+ if ((c2 = strchr(c, '/'))) {
+ *c2 = '\0';
+ c2++;
+ ast_string_field_set(o, tech, c);
+ ast_string_field_set(o, dest, c2);
+ } else {
+ ast_log(LOG_NOTICE, "Channel should be in form Tech/Dest at line %d of %s\n", lineno, o->fn);
+ }
+ } else if (!strcasecmp(line, "callerid")) {
+ char cid_name[80] = {0}, cid_num[80] = {0};
+ ast_callerid_split(c, cid_name, sizeof(cid_name), cid_num, sizeof(cid_num));
+ ast_string_field_set(o, cid_num, cid_num);
+ ast_string_field_set(o, cid_name, cid_name);
+ } else if (!strcasecmp(line, "application")) {
+ ast_string_field_set(o, app, c);
+ } else if (!strcasecmp(line, "data")) {
+ ast_string_field_set(o, data, c);
+ } else if (!strcasecmp(line, "maxretries")) {
+ if (sscanf(c, "%30d", &o->maxretries) != 1) {
+ ast_log(LOG_WARNING, "Invalid max retries at line %d of %s\n", lineno, o->fn);
+ o->maxretries = 0;
+ }
+ } else if (!strcasecmp(line, "codecs")) {
+ ast_format_cap_update_by_allow_disallow(o->capabilities, c, 1);
+ } else if (!strcasecmp(line, "context")) {
+ ast_string_field_set(o, context, c);
+ } else if (!strcasecmp(line, "extension")) {
+ ast_string_field_set(o, exten, c);
+ } else if (!strcasecmp(line, "priority")) {
+ if ((sscanf(c, "%30d", &o->priority) != 1) || (o->priority < 1)) {
+ ast_log(LOG_WARNING, "Invalid priority at line %d of %s\n", lineno, o->fn);
+ o->priority = 1;
+ }
+ } else if (!strcasecmp(line, "retrytime")) {
+ if ((sscanf(c, "%30d", &o->retrytime) != 1) || (o->retrytime < 1)) {
+ ast_log(LOG_WARNING, "Invalid retrytime at line %d of %s\n", lineno, o->fn);
+ o->retrytime = 300;
+ }
+ } else if (!strcasecmp(line, "waittime")) {
+ if ((sscanf(c, "%30d", &o->waittime) != 1) || (o->waittime < 1)) {
+ ast_log(LOG_WARNING, "Invalid waittime at line %d of %s\n", lineno, o->fn);
+ o->waittime = 45;
+ }
+ } else if (!strcasecmp(line, "retry")) {
+ o->retries++;
+ } else if (!strcasecmp(line, "startretry")) {
+ if (sscanf(c, "%30ld", &o->callingpid) != 1) {
+ ast_log(LOG_WARNING, "Unable to retrieve calling PID!\n");
+ o->callingpid = 0;
+ }
+ } else if (!strcasecmp(line, "endretry") || !strcasecmp(line, "abortretry")) {
+ o->callingpid = 0;
+ o->retries++;
+ } else if (!strcasecmp(line, "delayedretry")) {
+ } else if (!strcasecmp(line, "setvar") || !strcasecmp(line, "set")) {
+ char *c2 = c;
+
+ strsep(&c2, "=");
+ if (c2) {
+ struct ast_variable *var = ast_variable_new(c, c2, o->fn);
+
+ if (var) {
+ /*
+ * Always insert at the end, because some people
+ * want to treat the spool file as a script
+ */
+ struct ast_variable **tail = &o->vars;
+
+ while (*tail) {
+ tail = &(*tail)->next;
+ }
+ *tail = var;
+ }
+ } else {
+ ast_log(LOG_WARNING, "Malformed \"%s\" argument. Should be \"%s: variable=value\"\n", line, line);
+ }
+ } else if (!strcasecmp(line, "account")) {
+ ast_string_field_set(o, account, c);
+ } else if (!strcasecmp(line, "alwaysdelete")) {
+ ast_set2_flag(&o->options, ast_true(c), SPOOL_FLAG_ALWAYS_DELETE);
+ } else if (!strcasecmp(line, "archive")) {
+ ast_set2_flag(&o->options, ast_true(c), SPOOL_FLAG_ARCHIVE);
+ } else if (!strcasecmp(line, "early_media")) {
+ ast_set2_flag(&o->options, ast_true(c), SPOOL_FLAG_EARLY_MEDIA);
+ } else {
+ ast_log(LOG_WARNING, "Unknown keyword '%s' at line %d of %s\n", line, lineno, o->fn);
+ }
+}
+
+#define LINE_BUFFER_SIZE 1024
+
+static int apply_outgoing(struct outgoing *o, FILE *f)
+{
+ char buf[LINE_BUFFER_SIZE];
+ unsigned int lineno = 0;
+
+ while (fgets(buf, sizeof(buf), f)) {
+ size_t len = strlen(buf);
+
lineno++;
- /* Trim comments */
- c = buf;
- while ((c = strchr(c, '#'))) {
- if ((c == buf) || (*(c-1) == ' ') || (*(c-1) == '\t'))
- *c = '\0';
- else
- c++;
+
+ if (buf[len - 1] == '\n' || feof(f)) {
+ /* We have a line, parse it */
+ parse_line(buf, lineno, o);
+ continue;
}
- c = buf;
- while ((c = strchr(c, ';'))) {
- if ((c > buf) && (c[-1] == '\\')) {
- memmove(c - 1, c, strlen(c) + 1);
- c++;
- } else {
- *c = '\0';
+ /* Crazy long line, skip it */
+ ast_log(LOG_WARNING, "Skipping extremely long line at line %d of %s\n", lineno, o->fn);
+
+ /* Consume the rest of the problematic line */
+ while (fgets(buf, sizeof(buf), f)) {
+ len = strlen(buf);
+ if (buf[len - 1] == '\n' || feof(f)) {
break;
}
}
-
- /* Trim trailing white space */
- ast_trim_blanks(buf);
- if (ast_strlen_zero(buf)) {
- continue;
- }
- c = strchr(buf, ':');
- if (!c) {
- ast_log(LOG_NOTICE, "Syntax error at line %d of %s\n", lineno, o->fn);
- continue;
- }
- *c = '\0';
- c = ast_skip_blanks(c + 1);
-#if 0
- printf("'%s' is '%s' at line %d\n", buf, c, lineno);
-#endif
- if (!strcasecmp(buf, "channel")) {
- if ((c2 = strchr(c, '/'))) {
- *c2 = '\0';
- c2++;
- ast_string_field_set(o, tech, c);
- ast_string_field_set(o, dest, c2);
- } else {
- ast_log(LOG_NOTICE, "Channel should be in form Tech/Dest at line %d of %s\n", lineno, o->fn);
- }
- } else if (!strcasecmp(buf, "callerid")) {
- char cid_name[80] = {0}, cid_num[80] = {0};
- ast_callerid_split(c, cid_name, sizeof(cid_name), cid_num, sizeof(cid_num));
- ast_string_field_set(o, cid_num, cid_num);
- ast_string_field_set(o, cid_name, cid_name);
- } else if (!strcasecmp(buf, "application")) {
- ast_string_field_set(o, app, c);
- } else if (!strcasecmp(buf, "data")) {
- ast_string_field_set(o, data, c);
- } else if (!strcasecmp(buf, "maxretries")) {
- if (sscanf(c, "%30d", &o->maxretries) != 1) {
- ast_log(LOG_WARNING, "Invalid max retries at line %d of %s\n", lineno, o->fn);
- o->maxretries = 0;
- }
- } else if (!strcasecmp(buf, "codecs")) {
- ast_format_cap_update_by_allow_disallow(o->capabilities, c, 1);
- } else if (!strcasecmp(buf, "context")) {
- ast_string_field_set(o, context, c);
- } else if (!strcasecmp(buf, "extension")) {
- ast_string_field_set(o, exten, c);
- } else if (!strcasecmp(buf, "priority")) {
- if ((sscanf(c, "%30d", &o->priority) != 1) || (o->priority < 1)) {
- ast_log(LOG_WARNING, "Invalid priority at line %d of %s\n", lineno, o->fn);
- o->priority = 1;
- }
- } else if (!strcasecmp(buf, "retrytime")) {
- if ((sscanf(c, "%30d", &o->retrytime) != 1) || (o->retrytime < 1)) {
- ast_log(LOG_WARNING, "Invalid retrytime at line %d of %s\n", lineno, o->fn);
- o->retrytime = 300;
- }
- } else if (!strcasecmp(buf, "waittime")) {
- if ((sscanf(c, "%30d", &o->waittime) != 1) || (o->waittime < 1)) {
- ast_log(LOG_WARNING, "Invalid waittime at line %d of %s\n", lineno, o->fn);
- o->waittime = 45;
- }
- } else if (!strcasecmp(buf, "retry")) {
- o->retries++;
- } else if (!strcasecmp(buf, "startretry")) {
- if (sscanf(c, "%30ld", &o->callingpid) != 1) {
- ast_log(LOG_WARNING, "Unable to retrieve calling PID!\n");
- o->callingpid = 0;
- }
- } else if (!strcasecmp(buf, "endretry") || !strcasecmp(buf, "abortretry")) {
- o->callingpid = 0;
- o->retries++;
- } else if (!strcasecmp(buf, "delayedretry")) {
- } else if (!strcasecmp(buf, "setvar") || !strcasecmp(buf, "set")) {
- c2 = c;
- strsep(&c2, "=");
- if (c2) {
- var = ast_variable_new(c, c2, o->fn);
- if (var) {
- /* Always insert at the end, because some people want to treat the spool file as a script */
- if (last) {
- last->next = var;
- } else {
- o->vars = var;
- }
- last = var;
- }
- } else
- ast_log(LOG_WARNING, "Malformed \"%s\" argument. Should be \"%s: variable=value\"\n", buf, buf);
- } else if (!strcasecmp(buf, "account")) {
- ast_string_field_set(o, account, c);
- } else if (!strcasecmp(buf, "alwaysdelete")) {
- ast_set2_flag(&o->options, ast_true(c), SPOOL_FLAG_ALWAYS_DELETE);
- } else if (!strcasecmp(buf, "archive")) {
- ast_set2_flag(&o->options, ast_true(c), SPOOL_FLAG_ARCHIVE);
- } else if (!strcasecmp(buf, "early_media")) {
- ast_set2_flag(&o->options, ast_true(c), SPOOL_FLAG_EARLY_MEDIA);
- } else {
- ast_log(LOG_WARNING, "Unknown keyword '%s' at line %d of %s\n", buf, lineno, o->fn);
- }
}
- if (ast_strlen_zero(o->tech) || ast_strlen_zero(o->dest) || (ast_strlen_zero(o->app) && ast_strlen_zero(o->exten))) {
- ast_log(LOG_WARNING, "At least one of app or extension must be specified, along with tech and dest in file %s\n", o->fn);
+
+ if (ast_strlen_zero(o->tech)
+ || ast_strlen_zero(o->dest)
+ || (ast_strlen_zero(o->app) && ast_strlen_zero(o->exten))) {
+ ast_log(LOG_WARNING, "At least one of app or extension must be specified, "
+ "along with tech and dest in file %s\n", o->fn);
return -1;
}
return 0;
--
To view, visit https://gerrit.asterisk.org/5126
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I51bcf270c1b4347ba05b43f18dc2094c76f5d7b0
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list