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

diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index a0b668d8d03cb4228711c7ae26e6e09086e2abb0..c94513d2857a7a1d4c2ff6b3f308b9655f1c6e7d 100644 (file)
--- 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;
 }