[Asterisk-code-review] config.c: Fix potential memory corruption after [section](+). (asterisk[11])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: config.c: Fix potential memory corruption after [section](+).
......................................................................


config.c: Fix potential memory corruption after [section](+).

The memory corruption could happen if the [section](+) is the last section
in the file with trailing comments.  In this case process_text_line() has
left *last_cat is set to newcat and newcat is destroyed.

Change-Id: I0d1d999f553986f591becd000e7cc6ddfb978d93
---
M main/config.c
1 file changed, 23 insertions(+), 12 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/main/config.c b/main/config.c
index acbf08b..6edee56 100644
--- a/main/config.c
+++ b/main/config.c
@@ -1394,7 +1394,7 @@
 		 *		You can put a comma-separated list of categories and templates
 		 *		and '!' and '+' between parentheses, with obvious meaning.
 		 */
-		struct ast_category *newcat = NULL;
+		struct ast_category *newcat;
 		char *catname;
 
 		c = strchr(cur, ']');
@@ -1407,14 +1407,13 @@
 		if (*c++ != '(')
 			c = NULL;
 		catname = cur;
-		if (!(*cat = newcat = ast_category_new(catname,
-				S_OR(suggested_include_file, cfg->include_level == 1 ? "" : configfile),
-				lineno))) {
+		*cat = newcat = ast_category_new(catname,
+			S_OR(suggested_include_file, cfg->include_level == 1 ? "" : configfile),
+			lineno);
+		if (!newcat) {
 			return -1;
 		}
 		(*cat)->lineno = lineno;
-		*last_var = 0;
-		*last_cat = newcat;
 
 		/* add comments */
 		if (ast_test_flag(&flags, CONFIG_FLAG_WITHCOMMENTS))
@@ -1437,8 +1436,9 @@
 				} else if (!strcasecmp(cur, "+")) {
 					*cat = category_get(cfg, catname, 1);
 					if (!(*cat)) {
-						if (newcat)
+						if (newcat) {
 							ast_category_destroy(newcat);
+						}
 						ast_log(LOG_WARNING, "Category addition requested, but category '%s' does not exist, line %d of %s\n", catname, lineno, configfile);
 						return -1;
 					}
@@ -1461,8 +1461,19 @@
 				}
 			}
 		}
-		if (newcat)
-			ast_category_append(cfg, *cat);
+
+		/*
+		 * We need to set *last_cat to newcat here regardless.  If the
+		 * category is being appended to we have no place for trailing
+		 * comments on the appended category.  The appended category
+		 * may be in another file or it already has trailing comments
+		 * that we would then leak.
+		 */
+		*last_var = NULL;
+		*last_cat = newcat;
+		if (newcat) {
+			ast_category_append(cfg, newcat);
+		}
 	} else if (cur[0] == '#') { /* A directive - #include or #exec */
 		char *cur2;
 		char real_inclusion_name[256];
@@ -1618,7 +1629,7 @@
 			} else if ((v = ast_variable_new(cur, ast_strip(c), S_OR(suggested_include_file, cfg->include_level == 1 ? "" : configfile)))) {
 				v->lineno = lineno;
 				v->object = object;
-				*last_cat = 0;
+				*last_cat = NULL;
 				*last_var = v;
 				/* Put and reset comments */
 				v->blanklines = 0;
@@ -1658,8 +1669,8 @@
 	struct stat statbuf;
 	struct cache_file_mtime *cfmtime = NULL;
 	struct cache_file_include *cfinclude;
-	struct ast_variable *last_var = 0;
-	struct ast_category *last_cat = 0;
+	struct ast_variable *last_var = NULL;
+	struct ast_category *last_cat = NULL;
 	/*! Growable string buffer */
 	struct ast_str *comment_buffer = NULL;	/*!< this will be a comment collector.*/
 	struct ast_str *lline_buffer = NULL;	/*!< A buffer for stuff behind the ; */

-- 
To view, visit https://gerrit.asterisk.org/1420
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0d1d999f553986f591becd000e7cc6ddfb978d93
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>