[Home]

Summary:ASTERISK-23739: [patch]Segfault forwarding voicemail with ODBC storage enabled and realtime voicemail_data is used
Reporter:Stas Kobzar (stas.kobzar)Labels:patch
Date Opened:2014-05-13 15:12:28Date Closed:2020-01-20 09:11:59.000-0600
Priority:MinorRegression?
Status:Closed/CompleteComponents:Applications/app_voicemail Applications/app_voicemail/ODBC
Versions:11.9.0 13.18.4 Frequency of
Occurrence		Constant
Related
Issues:
is duplicated byASTERISK-25106 Segmentation fault in ast_variable_new when using app_voicemail with realtime
Environment:Attachments:( 0) asterisk_1.8.patch
( 1) asterisk_10.patch
( 2) asterisk_11.patch
( 3) asterisk_12.patch
( 4) backtrace-issue-23739.txt
( 5) full-issue-23739
( 6) valgrind-issue-23739.txt
Description:Hello,

Asterisk is crashing with with segfaul on VoiceMail application when using option 8 (forward message to another voicemail box). This happens when Asterisk is compiled with voicemail ODBC storage and voicemail_data realtime is enabled in extconfig.conf.

To reproduce:
- Asterisk configured with voicemail storage ODBC
- realtime voicemail_data family enabled
- Call to Voicemail and use option 8 to forward new voicemail. When prompted forward options press "2". Asterisk will crash with segfault.

This is happens because in function _copy_plain_file_ Asterisk is trying to load a voicemail message from realtime voicemail_data family:
{code}
if (ast_check_realtime("voicemail_data")) {
 var = ast_load_realtime("voicemail_data", "filename", frompath, SENTINEL);
 for (tmp = var; tmp; tmp = tmp->next) {
   if (!strcasecmp(tmp->name, "origmailbox")) {
     origmailbox = tmp->value;
}
 ...
{code}

If the record does not exists, following assignment will crash Asterisk:
{code}origmailbox = tmp->value;{code}

Patch attached.

Thank you,
Stas Kobzar


Comments:By: Stas Kobzar (stas.kobzar) 2014-05-13 15:20:11.319-0500

Patches for currently supported versions
By: Michael L. Young (elguero) 2014-05-13 17:30:45.098-0500

I just tried to reproduced this crash and was unable to do so.  Can you provide the backtrace please or any other information to help reproduce this issue?

Thank you
By: Stas Kobzar (stas.kobzar) 2014-05-14 09:31:37.754-0500

Hello Micheal,

Here is some details about the problem:
* Asterisk 11.9.0 is build with Voicemail Build Options: ODBC_STORAGE
* I have created realtime tables from source contrib directory:
** _contrib/realtime/mysql/voicemail.sql_
** _contrib/realtime/mysql/voicemail_data.sql_
** _contrib/realtime/mysql/voicemail_messages.sql_
* In my extconfig.conf I have following settings:
{code}
[settings]
voicemail => odbc,MySQL-astrealtime,voicemail_boxes
voicemail_data => odbc,MySQL-astrealtime,voicemail_messages
{code}
(In my case table _voicemail_data_ has name _voicemail_messages_)
* In my voicemail.conf I have following settings:
{code}
odbcstorage=MySQL-astrealtime
odbctable=voicemail_files
{code}
(In my case table _voicemail_messages_ has name _voicemail_files_)
* I dial to VoiceMailMain application
** select a message to play
** select option 8 to forward message and provide destination extension
** then I press 2 to finish message forwarding and Asterisk crashes

The problem is that when you have both voicemail storage ODBC and realtime voicemail_data enabled, function tries to find a record in voicemail_data before copying file (function copy_plain_file). And if record does not exists it crashes Asterisk when trying to get tmp->value which is not allocated IMHO.

When removing line {code}voicemail_data => odbc,MySQL-astrealtime,voicemail_messages{code} from extconfig.conf, it works fine.
That's why I set severity to "Minor". But still it crashes Asterisk and the patch I proposed is fixing that.

Please, find attached backtrace and valgrind logs. Also I am attaching full log from Asterisk. As you can see the last line before crash is:
{code}[2014-05-14 10:18:55] DEBUG[19934][C-00000000] res_config_odbc.c: Parameter 2 ('origmailbox') = '(null)'{code}

Best regards,
Stas

By: Stas Kobzar (stas.kobzar) 2014-05-14 09:32:40.551-0500

Backtrace and log
By: Michael L. Young (elguero) 2014-05-14 11:08:10.840-0500

Stas,

Thank you for that detailed information.  There is always something new to learn.  I have used ODBC for voice-mail storage for many years and fixed bugs here and there in app_voicemail.c.  I never noticed the ability to store, basically, the .txt information in the database.  It doesn't seem to be documented very well.

In looking at what this feature does and trying to understand its purpose, there is a bug here but I am not sure your patch is the correct solution.  Essentially, your patch turns off storing this data about a voice-mail message in the database if you have ODBC storage enabled and attempt to copy a file to another location.

What appears to be the problem is that when performing a cleanup of the mailbox after a user is done listening to their voice-mail, there is no cleanup being done to this voicemail_data (in your case voicemail_messages) table.  Therefore, when a new message comes in, it is unable to store the proper filename for that new message because filename is a key field and duplicate entries are not allowed in the database table.  When this is the case, the temporary filename of the file is being saved in the database table.  When copy_plain_file() is called, it is unable to find any information about that file being copied in the voicemail_data table which then creates the issue that you are having.

In summary, I see two issues that need to be solved.
* Properly remove the voice-mail record or update the filename field in the database table when a recording is deleted or moved to another folder during the closing of the mailbox
* Properly handle the condition when no data is returned from the database table based on the given filename
By: Stas Kobzar (stas.kobzar) 2014-05-15 08:21:11.768-0500

Hello Michael,
You are right. I will update my patch to follow these two issues.
Have a good day!
By: Rusty Newton (rnewton) 2014-06-25 08:51:55.559-0500

[~stas.kobzar] After you get your patch reworked, go ahead and follow the [Code Review|https://wiki.asterisk.org/wiki/display/AST/Code+Review] wiki page, check it against the coding guidelines and get the patch on reviewboard.
By: Friendly Automation (friendly-automation) 2020-01-20 09:12:00.964-0600

Change 13596 merged by Friendly Automation:
app_voicemail: Prevent crash when saving message with realtime voicemail

[https://gerrit.asterisk.org/c/asterisk/+/13596|https://gerrit.asterisk.org/c/asterisk/+/13596]
By: Friendly Automation (friendly-automation) 2020-01-20 09:20:52.055-0600

Change 13633 merged by Friendly Automation:
app_voicemail: Prevent crash when saving message with realtime voicemail

[https://gerrit.asterisk.org/c/asterisk/+/13633|https://gerrit.asterisk.org/c/asterisk/+/13633]
By: Friendly Automation (friendly-automation) 2020-01-20 09:21:48.584-0600

Change 13632 merged by Friendly Automation:
app_voicemail: Prevent crash when saving message with realtime voicemail

[https://gerrit.asterisk.org/c/asterisk/+/13632|https://gerrit.asterisk.org/c/asterisk/+/13632]
By: Friendly Automation (friendly-automation) 2020-01-20 09:32:41.340-0600

Change 13634 merged by Joshua Colp:
app_voicemail: Prevent crash when saving message with realtime voicemail

[https://gerrit.asterisk.org/c/asterisk/+/13634|https://gerrit.asterisk.org/c/asterisk/+/13634]