[asterisk-bugs] [JIRA] (ASTERISK-20858) app_minivm fails to clean up mkstemp files

[Sean Bright (JIRA)]

     [ https://issues.asterisk.org/jira/browse/ASTERISK-20858?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sean Bright updated ASTERISK-20858:
-----------------------------------

    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.

  was:
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.


> app_minivm fails to clean up mkstemp files
> ------------------------------------------
>
>                 Key: ASTERISK-20858
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-20858
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Applications/app_minivm
>            Reporter: Walter Doekes
>            Severity: Minor
>
> 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.



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