app_voicemail: always copy dynamic struct to avoid race condition

Voicemail email addresses can be corrupt or voicemail
emails can end up being sent to the wrong email address if asterisk is
reading voicemail.conf during a reload and processing an email at the
same time. This patch always copies the struct that would otherwise only
be copied once.

ASTERISK-24463 #close
Reported by: John Campbell
Tested by: Etienne Lessard
Tested by: Andrew Nagy
Change-Id: I3a0643813116da84e2617291903d0d489b7425fb

1 files changed, 78 insertions, 23 deletions

diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index a0b668d8d..c94513d28 100644
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -1725,13 +1725,14 @@ static struct ast_vm_user *find_user(struct ast_vm_user *ivm, const char *contex
 	}
 	if (cur) {
 		/* Make a copy, so that on a reload, we have no race */
-		if ((vmu = (ivm ? ivm : ast_malloc(sizeof(*vmu))))) {
+		if ((vmu = (ivm ? ivm : ast_calloc(1, sizeof(*vmu))))) {
+			ast_free(vmu->email);
+			ast_free(vmu->emailbody);
+			ast_free(vmu->emailsubject);
 			*vmu = *cur;
-			if (!ivm) {
-				vmu->email = ast_strdup(cur->email);
-				vmu->emailbody = ast_strdup(cur->emailbody);
-				vmu->emailsubject = ast_strdup(cur->emailsubject);
-			}
+			vmu->email = ast_strdup(cur->email);
+			vmu->emailbody = ast_strdup(cur->emailbody);
+			vmu->emailsubject = ast_strdup(cur->emailsubject);
 			ast_set2_flag(vmu, !ivm, VM_ALLOCED);
 			AST_LIST_NEXT(vmu, list) = NULL;
 		}
@@ -2009,17 +2010,18 @@ static int get_folder_by_name(const char *name)
 
 static void free_user(struct ast_vm_user *vmu)
 {
-	if (ast_test_flag(vmu, VM_ALLOCED)) {
-
-		ast_free(vmu->email);
-		vmu->email = NULL;
-
-		ast_free(vmu->emailbody);
-		vmu->emailbody = NULL;
+	if (!vmu) {
+		return;
+	}
 
-		ast_free(vmu->emailsubject);
-		vmu->emailsubject = NULL;
+	ast_free(vmu->email);
+	vmu->email = NULL;
+	ast_free(vmu->emailbody);
+	vmu->emailbody = NULL;
+	ast_free(vmu->emailsubject);
+	vmu->emailsubject = NULL;
 
+	if (ast_test_flag(vmu, VM_ALLOCED)) {
 		ast_free(vmu);
 	}
 }
@@ -2457,14 +2459,17 @@ static int __messagecount(const char *context, const char *mailbox, const char *
 		return 0;
 
 	/* We have to get the user before we can open the stream! */
+	memset(&vmus, 0, sizeof(vmus));
 	vmu = find_user(&vmus, context, mailbox);
 	if (!vmu) {
 		ast_log(AST_LOG_WARNING, "Couldn't find mailbox %s in context %s\n", mailbox, context);
+		free_user(vmu);
 		return -1;
 	} else {
 		/* No IMAP account available */
 		if (vmu->imapuser[0] == '\0') {
 			ast_log(AST_LOG_WARNING, "IMAP user not set for mailbox %s\n", vmu->mailbox);
+			free_user(vmu);
 			return -1;
 		}
 	}
@@ -2484,9 +2489,11 @@ static int __messagecount(const char *context, const char *mailbox, const char *
 	if (vms_p) {
 		ast_debug(3, "Returning before search - user is logged in\n");
 		if (fold == 0) { /* INBOX */
+			free_user(vmu);
 			return urgent ? vms_p->urgentmessages : vms_p->newmessages;
 		}
 		if (fold == 1) { /* Old messages */
+			free_user(vmu);
 			return vms_p->oldmessages;
 		}
 	}
@@ -2503,6 +2510,7 @@ static int __messagecount(const char *context, const char *mailbox, const char *
 	ret = init_mailstream(vms_p, fold);
 	if (!vms_p->mailstream) {
 		ast_log(AST_LOG_ERROR, "Houston we have a problem - IMAP mailstream is NULL\n");
+		free_user(vmu);
 		return -1;
 	}
 	if (ret == 0) {
@@ -2546,6 +2554,7 @@ static int __messagecount(const char *context, const char *mailbox, const char *
 		/*Freeing the searchpgm also frees the searchhdr*/
 		mail_free_searchpgm(&pgm);
 		ast_mutex_unlock(&vms_p->lock);
+		free_user(vmu);
 		vms_p->updated = 0;
 		return vms_p->vmArrayIndex;
 	} else {
@@ -2553,6 +2562,7 @@ static int __messagecount(const char *context, const char *mailbox, const char *
 		mail_ping(vms_p->mailstream);
 		ast_mutex_unlock(&vms_p->lock);
 	}
+	free_user(vmu);
 	return 0;
 }
 
@@ -6185,6 +6195,7 @@ static int msg_create_from_file(struct ast_vm_recording_data *recdata)
 		return -1;
 	}
 
+	memset(&svm, 0, sizeof(svm));
 	if (!(recipient = find_user(&svm, recdata->context, recdata->mailbox))) {
 		ast_log(LOG_ERROR, "No entry in voicemail config file for '%s@%s'\n", recdata->mailbox, recdata->context);
 		return -1;
@@ -6500,6 +6511,7 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
 	}
 
 	ast_debug(3, "Before find_user\n");
+	memset(&svm, 0, sizeof(svm));
 	if (!(vmu = find_user(&svm, context, ext))) {
 		ast_log(AST_LOG_WARNING, "No entry in voicemail config file for '%s'\n", ext);
 		pbx_builtin_setvar_helper(chan, "VMSTATUS", "FAILED");
@@ -6529,6 +6541,7 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
 	snprintf(tempfile, sizeof(tempfile), "%s%s/%s/temp", VM_SPOOL_DIR, vmu->context, ext);
 	if ((res = create_dirpath(tmpdir, sizeof(tmpdir), vmu->context, ext, "tmp"))) {
 		ast_log(AST_LOG_WARNING, "Failed to make directory (%s)\n", tempfile);
+		free_user(vmu);
 		ast_free(tmp);
 		return -1;
 	}
@@ -6672,9 +6685,9 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
 			}
 			ast_play_and_wait(chan, "transfer");
 			ast_channel_priority_set(chan, 0);
-			free_user(vmu);
 			pbx_builtin_setvar_helper(chan, "VMSTATUS", "USEREXIT");
 		}
+		free_user(vmu);
 		ast_free(tmp);
 		return OPERATOR_EXIT;
 	}
@@ -6708,6 +6721,7 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
 		res = inboxcount(ext_context, &newmsgs, &oldmsgs);
 		if (res < 0) {
 			ast_log(AST_LOG_NOTICE, "Can not leave voicemail, unable to count messages\n");
+			free_user(vmu);
 			ast_free(tmp);
 			return -1;
 		}
@@ -6718,6 +6732,7 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
 		 */
 			if (!(vms = create_vm_state_from_user(vmu))) {
 				ast_log(AST_LOG_ERROR, "Couldn't allocate necessary space\n");
+				free_user(vmu);
 				ast_free(tmp);
 				return -1;
 			}
@@ -6912,6 +6927,7 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
 							*cntx = '\0';
 							cntx++;
 						}
+						memset(&recipu, 0, sizeof(recipu));
 						if ((recip = find_user(&recipu, cntx, exten))) {
 							copy_message(chan, vmu, 0, msgnum, duration, recip, fmt, dir, flag, NULL);
 							free_user(recip);
@@ -10939,6 +10955,7 @@ static int vm_authenticate(struct ast_channel *chan, char *mailbox, int mailbox_
 		}
 
 		ast_debug(1, "Before find user for mailbox %s\n", mailbox);
+		memset(&vmus, 0, sizeof(vmus));
 		vmu = find_user(&vmus, context, mailbox);
 		if (vmu && (vmu->password[0] == '\0' || (vmu->password[0] == '-' && vmu->password[1] == '\0'))) {
 			/* saved password is blank, so don't bother asking */
@@ -10946,10 +10963,12 @@ static int vm_authenticate(struct ast_channel *chan, char *mailbox, int mailbox_
 		} else {
 			if (ast_streamfile(chan, vm_password, ast_channel_language(chan))) {
 				ast_log(AST_LOG_WARNING, "Unable to stream password file\n");
+				free_user(vmu);
 				return -1;
 			}
 			if (ast_readstring(chan, password, sizeof(password) - 1, 2000, 10000, "#") < 0) {
 				ast_log(AST_LOG_WARNING, "Unable to read password\n");
+				free_user(vmu);
 				return -1;
 			} else if (password[0] == '*') {
 				/* user entered '*' */
@@ -10957,11 +10976,13 @@ static int vm_authenticate(struct ast_channel *chan, char *mailbox, int mailbox_
 				if (ast_exists_extension(chan, ast_channel_context(chan), "a", 1,
 					S_COR(ast_channel_caller(chan)->id.number.valid, ast_channel_caller(chan)->id.number.str, NULL))) {
 					mailbox[0] = '*';
+					free_user(vmu);
 					return -1;
 				}
 				ast_verb(4, "Jump to extension 'a' failed; setting mailbox and user to NULL\n");
 				mailbox[0] = '\0';
 				/* if the password entered was '*', do not let a user mailbox be created if the extension 'a' is not defined */
+				free_user(vmu);
 				vmu = NULL;
 			}
 		}
@@ -10982,6 +11003,7 @@ static int vm_authenticate(struct ast_channel *chan, char *mailbox, int mailbox_
 			if (skipuser || logretries >= max_logins) {
 				if (ast_streamfile(chan, "vm-incorrect", ast_channel_language(chan))) {
 					ast_log(AST_LOG_WARNING, "Unable to stream incorrect message\n");
+					free_user(vmu);
 					return -1;
 				}
 			} else {
@@ -10989,16 +11011,20 @@ static int vm_authenticate(struct ast_channel *chan, char *mailbox, int mailbox_
 					adsi_login(chan);
 				if (ast_streamfile(chan, "vm-incorrect-mailbox", ast_channel_language(chan))) {
 					ast_log(AST_LOG_WARNING, "Unable to stream incorrect mailbox message\n");
+					free_user(vmu);
 					return -1;
 				}
 			}
-			if (ast_waitstream(chan, ""))	/* Channel is hung up */
+			if (ast_waitstream(chan, "")) {	/* Channel is hung up */
+				free_user(vmu);
 				return -1;
+			}
 		}
 	}
 	if (!valid && (logretries >= max_logins)) {
 		ast_stopstream(chan);
 		ast_play_and_wait(chan, "vm-goodbye");
+		free_user(vmu);
 		return -1;
 	}
 	if (vmu && !skipuser) {
@@ -11106,6 +11132,8 @@ play_msg_cleanup:
 	}
 #endif
 
+	free_user(vmu);
+
 	return res;
 }
 
@@ -12301,7 +12329,7 @@ AST_TEST_DEFINE(test_voicemail_vmuser)
 
 static int vm_box_exists(struct ast_channel *chan, const char *data) 
 {
-	struct ast_vm_user svm;
+	struct ast_vm_user svm, *vmu;
 	char *context, *box;
 	AST_DECLARE_APP_ARGS(args,
 		AST_APP_ARG(mbox);
@@ -12331,8 +12359,10 @@ static int vm_box_exists(struct ast_channel *chan, const char *data)
 		context++;
 	}
 
-	if (find_user(&svm, context, args.mbox)) {
+	vmu = find_user(&svm, context, args.mbox);
+	if (vmu) {
 		pbx_builtin_setvar_helper(chan, "VMBOXEXISTSSTATUS", "SUCCESS");
+		free_user(vmu);
 	} else
 		pbx_builtin_setvar_helper(chan, "VMBOXEXISTSSTATUS", "FAILED");
 
@@ -12341,7 +12371,7 @@ static int vm_box_exists(struct ast_channel *chan, const char *data)
 
 static int acf_mailbox_exists(struct ast_channel *chan, const char *cmd, char *args, char *buf, size_t len)
 {
-	struct ast_vm_user svm;
+	struct ast_vm_user svm, *vmu;
 	AST_DECLARE_APP_ARGS(arg,
 		AST_APP_ARG(mbox);
 		AST_APP_ARG(context);
@@ -12360,7 +12390,10 @@ static int acf_mailbox_exists(struct ast_channel *chan, const char *cmd, char *a
 		ast_log(AST_LOG_WARNING, "MAILBOX_EXISTS is deprecated.  Please use ${VM_INFO(%s,exists)} instead.\n", args);
 	}
 
-	ast_copy_string(buf, find_user(&svm, ast_strlen_zero(arg.context) ? "default" : arg.context, arg.mbox) ? "1" : "0", len);
+	vmu = find_user(&svm, ast_strlen_zero(arg.context) ? "default" : arg.context, arg.mbox);
+	ast_copy_string(buf, vmu ? "1" : "0", len);
+	free_user(vmu);
+
 	return 0;
 }
 
@@ -12396,10 +12429,12 @@ static int acf_vm_info(struct ast_channel *chan, const char *cmd, char *args, ch
 		return -1;
 	}
 
+	memset(&svm, 0, sizeof(svm));
 	vmu = find_user(&svm, context, mailbox);
 
 	if (!strncasecmp(arg.attribute, "exists", 5)) {
 		ast_copy_string(buf, vmu ? "1" : "0", len);
+		free_user(vmu);
 		return 0;
 	}
 
@@ -12428,13 +12463,16 @@ static int acf_vm_info(struct ast_channel *chan, const char *cmd, char *args, ch
 			res = messagecount(mailbox_id, arg.folder);
 			if (res < 0) {
 				ast_log(LOG_ERROR, "Unable to retrieve message count for mailbox %s\n", arg.mailbox_context);
+				free_user(vmu);
 				return -1;
 			}
 			snprintf(buf, len, "%d", res);
 		} else {
 			ast_log(LOG_ERROR, "Unknown attribute '%s' for VM_INFO\n", arg.attribute);
+			free_user(vmu);
 			return -1;
 		}
+		free_user(vmu);
 	}
 
 	return 0;
@@ -14248,6 +14286,7 @@ AST_TEST_DEFINE(test_voicemail_msgcount)
 	}
 #endif
 
+	memset(&svm, 0, sizeof(svm));
 	if (!(vmu = find_user(&svm, testcontext, testmailbox)) &&
 		!(vmu = find_or_create(testcontext, testmailbox))) {
 		ast_test_status_update(test, "Cannot create vmu structure\n");
@@ -14277,6 +14316,7 @@ AST_TEST_DEFINE(test_voicemail_msgcount)
 #ifdef IMAP_STORAGE
 				chan = ast_channel_unref(chan);
 #endif
+				free_user(vmu);
 				return AST_TEST_FAIL;
 			}
 		}
@@ -14360,6 +14400,7 @@ AST_TEST_DEFINE(test_voicemail_msgcount)
 			syserr > 0 ? strerror(syserr) : "unable to fork()");
 	}
 
+	free_user(vmu);
 	return res;
 }
 
@@ -14469,6 +14510,7 @@ AST_TEST_DEFINE(test_voicemail_notify_endl)
 		}
 	}
 	fclose(file);
+	free_user(vmu);
 	return res;
 }
 
@@ -14629,6 +14671,7 @@ AST_TEST_DEFINE(test_voicemail_vm_info)
 	}
 
 	chan = ast_channel_unref(chan);
+	free_user(vmu);
 	return res;
 }
 #endif /* defined(TEST_FRAMEWORK) */
@@ -15020,8 +15063,10 @@ static int advanced_options(struct ast_channel *chan, struct ast_vm_user *vmu, s
 			ast_config_destroy(msg_cfg);
 			return res;
 		} else {
-			struct ast_vm_user vmu2;
-			if (find_user(&vmu2, vmu->context, num)) {
+			struct ast_vm_user vmu2, *vmu3;
+			memset(&vmu2, 0, sizeof(vmu2));
+			vmu3 = find_user(&vmu2, vmu->context, num);
+			if (vmu3) {
 				struct leave_vm_options leave_options;
 				char mailbox[AST_MAX_EXTENSION * 2 + 2];
 				snprintf(mailbox, sizeof(mailbox), "%s@%s", num, vmu->context);
@@ -15034,6 +15079,7 @@ static int advanced_options(struct ast_channel *chan, struct ast_vm_user *vmu, s
 				if (!res)
 					res = 't';
 				ast_config_destroy(msg_cfg);
+				free_user(vmu3);
 				return res;
 			} else {
 				/* Sender has no mailbox, can't reply */
@@ -15528,11 +15574,13 @@ static struct ast_vm_mailbox_snapshot *vm_mailbox_snapshot_create(const char *ma
 
 	if (!(mailbox_snapshot = ast_calloc(1, sizeof(*mailbox_snapshot)))) {
 		ast_log(AST_LOG_ERROR, "Failed to allocate memory for mailbox snapshot\n");
+		free_user(vmu);
 		return NULL;
 	}
 
 	if (!(mailbox_snapshot->snapshots = ast_calloc(ARRAY_LEN(mailbox_folders), sizeof(*mailbox_snapshot->snapshots)))) {
 		ast_free(mailbox_snapshot);
+		free_user(vmu);
 		return NULL;
 	}
 
@@ -15593,6 +15641,7 @@ snapshot_cleanup:
 	}
 #endif
 
+	free_user(vmu);
 	return mailbox_snapshot;
 }
 
@@ -15747,6 +15796,7 @@ static int vm_msg_forward(const char *from_mailbox,
 
 	if (!(to_vmu = find_user(&to_vmus, to_context, to_mailbox))) {
 		ast_log(LOG_WARNING, "Can't find voicemail user to forward to (%s@%s)\n", to_mailbox, to_context);
+		free_user(vmu);
 		return -1;
 	}
 
@@ -15827,6 +15877,8 @@ vm_forward_cleanup:
 		notify_new_state(to_vmu);
 	}
 
+	free_user(vmu);
+	free_user(to_vmu);
 	return res;
 }
 
@@ -15930,6 +15982,7 @@ vm_move_cleanup:
 		notify_new_state(vmu);
 	}
 
+	free_user(vmu);
 	return res;
 }
 
@@ -16027,6 +16080,7 @@ vm_remove_cleanup:
 		notify_new_state(vmu);
 	}
 
+	free_user(vmu);
 	return res;
 }
 
@@ -16140,6 +16194,7 @@ play2_msg_cleanup:
 		notify_new_state(vmu);
 	}
 
+	free_user(vmu);
 	return res;
 }