[Home]

Summary:ASTERISK-20858: app_minivm fails to clean up mkstemp files
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2013-01-03 08:33:27.000-0600Date Closed:2017-08-29 05:09:50
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_minivm
Versions:Frequency of
Occurrence
Related
Issues:
is related toASTERISK-17133 [patch] minivm: when sending mail and using volgain
Environment:Attachments:
Description:Matt recently touched this code and removed a comment about leaking fd's and files.

{noformat}
tmpfd = mkstemp(newtmp);
if (tmpfd < 0) {
       ast_log(LOG_WARNING, "Failed to create temporary file for volgain: %d\n", errno);
       ast_free(str1);
       ast_free(str2);
       return -1;
}
snprintf(tmpcmd, sizeof(tmpcmd), "sox -v %.4f %s.%s %s.%s", vmu->volgain, filename, format, newtmp, format);
ast_safe_system(tmpcmd);
{noformat}

The fd leaks may be gone, but this looks like:

(a) file leaks
(b) misuse of mkstemp by appending format to the filename

First we have mkstemp create an actual file "/tmp/ABCDEF" and then we write to "/tmp/ABCDEF.wav". No one cares about the original "/tmp/ABCDEF" on disk, and by writing to "/tmp/ABCDEF.wav" you're ignoring most of the functionality of mkstemp -- being secure and not overwriting existing files.

Lastly, no one ever clears up fname (finalfilename.format), resulting in a second file leak.
Comments:By: Matt Jordan (mjordan) 2013-01-03 08:47:28.120-0600

So, my only defense here is that I didn't introduce (a) or (b) :-)

It looks like what you described is actual the original intent of the code - to use mkstemp for the unique string of characters it produces as an input to {{sox}}, as opposed to actually using the file it creates. You're right that it probably shouldn't do that and that it should at least clean up the 0 byte file it just created on disk.

I don't think you're very likely to have a filename collision by appending '.wav' to ABCDEF when you call {{sox}}, but it  is possible.

By: Friendly Automation (friendly-automation) 2017-08-29 05:09:51.352-0500

Change 6312 merged by Jenkins2:
voicemail: Fix various abuses of mkstemp

[https://gerrit.asterisk.org/6312|https://gerrit.asterisk.org/6312]

By: Friendly Automation (friendly-automation) 2017-08-29 05:17:48.622-0500

Change 6314 merged by Jenkins2:
voicemail: Fix various abuses of mkstemp

[https://gerrit.asterisk.org/6314|https://gerrit.asterisk.org/6314]

By: Friendly Automation (friendly-automation) 2017-08-29 05:18:04.712-0500

Change 6313 merged by Jenkins2:
voicemail: Fix various abuses of mkstemp

[https://gerrit.asterisk.org/6313|https://gerrit.asterisk.org/6313]

By: Friendly Automation (friendly-automation) 2017-08-29 05:19:25.273-0500

Change 6315 merged by Jenkins2:
voicemail: Fix various abuses of mkstemp

[https://gerrit.asterisk.org/6315|https://gerrit.asterisk.org/6315]