[Asterisk-code-review] app directory: Fix crash when using the alias option 'a'. (asterisk[master])

[Mark Michelson]

Mark Michelson has submitted this change and it was merged.

Change subject: app_directory: Fix crash when using the alias option 'a'.
......................................................................


app_directory: Fix crash when using the alias option 'a'.

The voicemail.conf mailbox key/value pair is defined as:
<mailbox>=[<password>[,<full-name>[,<email>[,<pager>[,<options>]]]]]
Where all fields in the value including the field values are optional.

Since the parsing code for the mailbox key/value pair is sloppy, this
patch tightens the parsing for the directory information.

* Renamed the 'pos' and 'bufptr' variables to 'name' and 'options'
respectively in search_directory_sub().  Those names make more sense.

* Made sure that search_directory_sub() is dealing with the voicemail.conf
mailbox options field if it even exists when looking for the 'hidefromdir'
and 'alias' options.

* Fix crash if a voicemail.conf mailbox is just
<mailbox>=<password>,<name> when the 'a' option is used.  If there were no
fields after the name then the 'options' pointer was not checked for NULL.

* Fix users.conf alias processing if the 'a' option is used.  The wrong
variable was used.

ASTERISK-25087 #close
Reported by: Chet Stevens

Change-Id: I86052ea77307beddddba5279824d39dc0d593374
---
M apps/app_directory.c
1 file changed, 27 insertions(+), 19 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/apps/app_directory.c b/apps/app_directory.c
index c872886..c32bccb 100644
--- a/apps/app_directory.c
+++ b/apps/app_directory.c
@@ -518,7 +518,9 @@
 			/* Skip hidden */
 			continue;
 		}
-		ast_str_set(&tmp, 0, "no-password,%s", S_OR(fullname, ""));
+
+		/* password,Full Name,email,pager,options */
+		ast_str_set(&tmp, 0, "no-password,%s,,,", S_OR(fullname, ""));
 		if (ast_variable_retrieve(rtdata, mailbox, "alias")) {
 			for (alias = ast_variable_browse(rtdata, mailbox); alias; alias = alias->next) {
 				if (!strcasecmp(alias->name, "alias")) {
@@ -602,7 +604,10 @@
 {
 	struct ast_variable *v;
 	struct ast_str *buf = ast_str_thread_get(&commonbuf, 100);
-	char *pos, *bufptr, *cat, *alias;
+	char *name;
+	char *options;
+	char *alias;
+	char *cat;
 	struct directory_item *item;
 	int res;
 
@@ -613,33 +618,36 @@
 	ast_debug(2, "Pattern: %s\n", ext);
 
 	for (v = ast_variable_browse(vmcfg, context); v; v = v->next) {
-
-		/* Ignore hidden */
-		if (strcasestr(v->value, "hidefromdir=yes")) {
-			continue;
-		}
-
 		ast_str_set(&buf, 0, "%s", v->value);
-		bufptr = ast_str_buffer(buf);
+		options = ast_str_buffer(buf);
 
 		/* password,Full Name,email,pager,options */
-		strsep(&bufptr, ",");
-		pos = strsep(&bufptr, ",");
+		strsep(&options, ",");          /* Skip password */
+		name = strsep(&options, ",");   /* Save full name */
+		strsep(&options, ",");          /* Skip email */
+		strsep(&options, ",");          /* Skip pager */
+		/* options is now the options field if it exists. */
 
-		/* No name to compare against */
-		if (ast_strlen_zero(pos)) {
+		if (options && strcasestr(options, "hidefromdir=yes")) {
+			/* Ignore hidden */
+			continue;
+		}
+		if (ast_strlen_zero(name)) {
+			/* No name to compare against */
 			continue;
 		}
 
 		res = 0;
 		if (ast_test_flag(&flags, OPT_LISTBYLASTNAME)) {
-			res = check_match(&item, context, pos, v->name, ext, 0 /* use_first_name */);
+			res = check_match(&item, context, name, v->name, ext, 0 /* use_first_name */);
 		}
 		if (!res && ast_test_flag(&flags, OPT_LISTBYFIRSTNAME)) {
-			res = check_match(&item, context, pos, v->name, ext, 1 /* use_first_name */);
+			res = check_match(&item, context, name, v->name, ext, 1 /* use_first_name */);
 		}
-		if (!res && ast_test_flag(&flags, OPT_ALIAS) && (alias = strcasestr(bufptr, "alias="))) {
+		if (!res && ast_test_flag(&flags, OPT_ALIAS)
+			&& options && (alias = strcasestr(options, "alias="))) {
 			char *a;
+
 			ast_debug(1, "Found alias: %s\n", alias);
 			while ((a = strsep(&alias, "|"))) {
 				if (!strncasecmp(a, "alias=", 6)) {
@@ -683,9 +691,9 @@
 				res = check_match(&item, context, position, cat, ext, 1 /* use_first_name */);
 			}
 			if (!res && ast_test_flag(&flags, OPT_ALIAS)) {
-				struct ast_variable *alias;
-				for (alias = ast_variable_browse(ucfg, cat); alias; alias = alias->next) {
-					if (!strcasecmp(v->name, "alias") && (res = check_match(&item, context, v->value, cat, ext, 1))) {
+				for (v = ast_variable_browse(ucfg, cat); v; v = v->next) {
+					if (!strcasecmp(v->name, "alias")
+						&& (res = check_match(&item, context, v->value, cat, ext, 1))) {
 						break;
 					}
 				}

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I86052ea77307beddddba5279824d39dc0d593374
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>