There is a memory leak defect at line 120 of the file /WavPack/cli/caff_write.c.

[LuMingYinDetect]

I found a suspected memory leak defect at line 120 of the file /WavPack/cli/caff_write.c using a static analysis tool. Below is the link to the detailed description of this defect:https://github.com/LuMingYinDetect/WavPack_defects/blob/main/WavPack_detect_1.md

[dbry]

Hi, and thanks for reporting this and the detailed analysis!

You are correct that this is a potential memory leak, assuming that there is an error writing the output file. There is a similar condition in the same module with the pointer new_channel_order a few lines down. In fact, the WavPack command-line programs have many potential memory leaks on fatal error conditions (see issues #165, #124, #70) and you will find discussions there about why I don’t feel that freeing memory before exiting a process is important, or even always a good idea. You can also find endless debates about this on the web.

In this particular case I may try to create a fix if it does not become too messy, However, you say that such a memory leak could “potentially be exploited maliciously to cause resource exhaustion and denial of service attacks”. Could you please explain to me how that might work?

[LuMingYinDetect]

Hi, and thanks for reporting this and the detailed analysis!

You are correct that this is a potential memory leak, assuming that there is an error writing the output file. There is a similar condition in the same module with the pointer new_channel_order a few lines down. In fact, the WavPack command-line programs have many potential memory leaks on fatal error conditions (see issues #165, #124, #70) and you will find discussions there about why I don’t feel that freeing memory before exiting a process is important, or even always a good idea. You can also find endless debates about this on the web.

In this particular case I may try to create a fix if it does not become too messy, However, you say that such a memory leak could “potentially be exploited maliciously to cause resource exhaustion and denial of service attacks”. Could you please explain to me how that might work?

Hello! The possible program crash or denial of service attack I mentioned in the description refers to the potential problems caused by memory leaks, not the flaw itself. I apologize for my inappropriate expression! Because I am engaged in static analysis, static analysis only analyzes memory leak defects from the perspective of code structure, and does not actually run the code to observe whether the defect will cause serious problems.

[LuMingYinDetect]

Hi, and thanks for reporting this and the detailed analysis!

You are correct that this is a potential memory leak, assuming that there is an error writing the output file. There is a similar condition in the same module with the pointer new_channel_order a few lines down. In fact, the WavPack command-line programs have many potential memory leaks on fatal error conditions (see issues #165, #124, #70) and you will find discussions there about why I don’t feel that freeing memory before exiting a process is important, or even always a good idea. You can also find endless debates about this on the web.

In this particular case I may try to create a fix if it does not become too messy, However, you say that such a memory leak could “potentially be exploited maliciously to cause resource exhaustion and denial of service attacks”. Could you please explain to me how that might work?

And I noticed that similarly, for the variable "channel_identities," the memory space pointed to by this variable was released at line 99 of the program. Considering this, I believed it was necessary to release the dynamically allocated memory area pointed to by this variable before returning at line 120.

[dbry]

Hi, and thanks for reporting this and the detailed analysis!
You are correct that this is a potential memory leak, assuming that there is an error writing the output file. There is a similar condition in the same module with the pointer new_channel_order a few lines down. In fact, the WavPack command-line programs have many potential memory leaks on fatal error conditions (see issues #165, #124, #70) and you will find discussions there about why I don’t feel that freeing memory before exiting a process is important, or even always a good idea. You can also find endless debates about this on the web.
In this particular case I may try to create a fix if it does not become too messy, However, you say that such a memory leak could “potentially be exploited maliciously to cause resource exhaustion and denial of service attacks”. Could you please explain to me how that might work?

And I noticed that similarly, for the variable "channel_identities," the memory space pointed to by this variable was released at line 99 of the program. Considering this, I believed it was necessary to release the dynamically allocated memory area pointed to by this variable before returning at line 120.

Yes, you are completely correct. However, there are 10 other places with the same problem, and there's the same thing with the other pointer new_channel_order, which makes the fix rather ugly, especially considering that it's completely unnecessary, as I explained above and in those other issues.

However, I have checked in a fix for this. Could you be so kind as to check whether I didn't miss anything as it's rather extensive? Thanks!

[LuMingYinDetect]

Hi, and thanks for reporting this and the detailed analysis!
You are correct that this is a potential memory leak, assuming that there is an error writing the output file. There is a similar condition in the same module with the pointer new_channel_order a few lines down. In fact, the WavPack command-line programs have many potential memory leaks on fatal error conditions (see issues #165, #124, #70) and you will find discussions there about why I don’t feel that freeing memory before exiting a process is important, or even always a good idea. You can also find endless debates about this on the web.
In this particular case I may try to create a fix if it does not become too messy, However, you say that such a memory leak could “potentially be exploited maliciously to cause resource exhaustion and denial of service attacks”. Could you please explain to me how that might work?

And I noticed that similarly, for the variable "channel_identities," the memory space pointed to by this variable was released at line 99 of the program. Considering this, I believed it was necessary to release the dynamically allocated memory area pointed to by this variable before returning at line 120.

Yes, you are completely correct. However, there are 10 other places with the same problem, and there's the same thing with the other pointer new_channel_order, which makes the fix rather ugly, especially considering that it's completely unnecessary, as I explained above and in those other issues.

However, I have checked in a fix for this. Could you be so kind as to check whether I didn't miss anything as it's rather extensive? Thanks!

Thank you for your quick response! I have re-run the static analysis tool on your fixed WavPack, and the vast majority of memory leak issues have been addressed by your fixes! However, I also noticed that you seem to have missed fixing a memory leak related to new_channel_order, and there are also several other variables with memory leaks (you can decide whether to fix them based on your discretion). I have generated reports for these issues in HTML format and uploaded them to my GitHub link (there are only 5 issues related to memory leaks; you can ignore other types of issues as they are mostly false positives). You just need to download this HTML file locally and double-click to open it for review! Here is the link to the issue reports: https://github.com/LuMingYinDetect/WavPack_defects/blob/main/WavPack_Memory_Leak.html