[asterisk-bugs] [JIRA] (ASTERISK-23713) Voicemail on FS overwrites last message when last index = MSGLIMIT

[Miguel Tavares (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-23713?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=217927#comment-217927 ] 

Miguel Tavares edited comment on ASTERISK-23713 at 5/5/14 1:37 PM:
-------------------------------------------------------------------

Here is the proper patch (not just a diff). I'll try to follow the code review (so that I also get to learn how to do that). EDIT: my password doesn't work with the review board so this might take some time.

{noformat}
Index: apps/app_voicemail.c
===================================================================
--- apps/app_voicemail.c        (revision 413303)
+++ apps/app_voicemail.c        (working copy)
@@ -4431,13 +4431,14 @@
 static int last_message_index(struct ast_vm_user *vmu, char *dir)
 {
        int x;
-       unsigned char map[MAXMSGLIMIT] = "";
+    unsigned char map[MAXMSGLIMIT];
        DIR *msgdir;
        struct dirent *msgdirent;
        int msgdirint;
        char extension[4];
        int stopcount = 0;
 
+    memset (map, 0, sizeof (map));
        /* Reading the entire directory into a file map scales better than
         * doing a stat repeatedly on a predicted sequence.  I suspect this
         * is partially due to stat(2) internally doing a readdir(2) itself to
@@ -4455,7 +4456,10 @@
        }
        closedir(msgdir);
 
-       for (x = 0; x < vmu->maxmsg; x++) {
+    /* If  by change there's a msg named msg9999 then this function
+     * will return 9998 and then the msg9999 will be overwritten.
+     * It's a unlikely scenario, but one that can happen... */
+    for (x = 0; x < MAXMSGLIMIT; x++) {
                if (map[x] == 1) {
                        stopcount--;
                } else if (map[x] == 0 && !stopcount) {
{noformat}


was (Author: mtavares):
Here is the proper patch (not just a diff). I'll try to follow the code review (so that I also get to learn how to do that).

{noformat}
Index: apps/app_voicemail.c
===================================================================
--- apps/app_voicemail.c        (revision 413303)
+++ apps/app_voicemail.c        (working copy)
@@ -4431,13 +4431,14 @@
 static int last_message_index(struct ast_vm_user *vmu, char *dir)
 {
        int x;
-       unsigned char map[MAXMSGLIMIT] = "";
+    unsigned char map[MAXMSGLIMIT];
        DIR *msgdir;
        struct dirent *msgdirent;
        int msgdirint;
        char extension[4];
        int stopcount = 0;
 
+    memset (map, 0, sizeof (map));
        /* Reading the entire directory into a file map scales better than
         * doing a stat repeatedly on a predicted sequence.  I suspect this
         * is partially due to stat(2) internally doing a readdir(2) itself to
@@ -4455,7 +4456,10 @@
        }
        closedir(msgdir);
 
-       for (x = 0; x < vmu->maxmsg; x++) {
+    /* If  by change there's a msg named msg9999 then this function
+     * will return 9998 and then the msg9999 will be overwritten.
+     * It's a unlikely scenario, but one that can happen... */
+    for (x = 0; x < MAXMSGLIMIT; x++) {
                if (map[x] == 1) {
                        stopcount--;
                } else if (map[x] == 0 && !stopcount) {
{noformat}

> Voicemail on FS overwrites last message when last index = MSGLIMIT
> ------------------------------------------------------------------
>
>                 Key: ASTERISK-23713
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-23713
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Applications/app_voicemail
>    Affects Versions: SVN, 12.2.0
>         Environment: app_voicemail delivery on filesystem.
> Any asterisk supported platform is probably affected.
>            Reporter: Miguel Tavares
>            Assignee: Miguel Tavares
>
> When leaving a voicemail the function last_message_index will stop at index only until the default or configured max messages (vmu->maxmsg) although it should continue until MAXMSGLIMIT.
> Example, let's say the asterisk admin set the maximum messages to 10, the user received 10 messages and then deleted 9 apart from the last one. In the filesystem the files msg0010.txt and msg0010.wav will be present.
> The last_message_index will detect that that's the only message available but then  when finding out what's the index it will stop at 9 (the configure maximum when counting from 0) and return that as the last message index, making he voicemail app to overwrite the previous existing last message.
> Possible fix, in app_voicemail.c
> {noformat}
> 112a113
> > #include <string.h>
> 4434c4435
> <       unsigned char map[MAXMSGLIMIT] = "";
> ---
> >       unsigned char map[MAXMSGLIMIT];
> 4440a4442
> >     memset (map, 0, sizeof(map)); //set the memory to 0 for Safety sake.
> 4458c4460
> <       for (x = 0; x < vmu->maxmsg; x++) {
> ---
> >       for (x = 0; x < MAXMSGLIMIT; x++) {
> {noformat}
> The important change is running the for cycle until MAXMSGLIMIT but as far as I know there's no guarantee that memory allocated in the stack is zeroed so that's why I also included the memset call.



--
This message was sent by Atlassian JIRA
(v6.2#6252)