[asterisk-dev] [Code Review] 4126: app_voicemail: Fix unchecked bounds of myArray in IMAP_STORAGE.

[wdoekes]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4126/
-----------------------------------------------------------

(Updated Oct. 29, 2014, 3:40 p.m.)


Review request for Asterisk Developers.


Changes
-------

Removed LOG_ERROR about not enough mem.


Bugs: ASTERISK-24190
    https://issues.asterisk.org/jira/browse/ASTERISK-24190


Repository: Asterisk


Description
-------

In update_messages_by_imapuser(), messages were appended without checking bounds:

    vms->msgArray[vms->vmArrayIndex++] = number;

This patch ensures that there is enough room.


However, I did find quirky usage of thread local storage which I couldn't explain. Perhaps someone else can shed some light on the XXX's that I left in the code:

- vms is thread-local, so it may not need to be freed. But on line 3033, it is overwritten if (strcmp(vms_p->imapuser, vmu->imapuser) || strcmp(vms_p->username, vmu->mailbox))
  (or should it be freed in vmstate_delete?)

- in vmstate_insert, an alternative mailbox overwrites the supplied one, but no msgArray copying is done. That can't be right.


Diffs (updated)
-----

  /branches/1.8/apps/app_voicemail.c 426593 

Diff: https://reviewboard.asterisk.org/r/4126/diff/


Testing
-------

The reporter -- Nick Adams -- has run this patch in production for a number of months now, without issues.


Thanks,

wdoekes

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141029/ae62ea9e/attachment-0001.html>