Video recording file path including dot char leads to a heap failure.

[cleanner]

When I set the VM with a speical name called "abc.def", it leads to a heap failure after I start the video recording.

It's because it finally free a string which can not be released by RTStrFree(), which will lead to a heap failure.

I found that the error comes from "src\VBox\Main\src-client\VideoRec.cpp", line 950.

---

944: char *pszAbsPath = RTPathAbsDup(com::Utf8Str(pCfg->File.strName).c_str());

945: AssertPtrReturn(pszAbsPath, VERR_NO_MEMORY);

947: RTPathStripSuffix(pszAbsPath);

948: AssertPtrReturn(pszAbsPath, VERR_INVALID_PARAMETER);

950: char *pszSuff = RTPathSuffix(pszAbsPath); /* it should be "char *pszSuff = NULL"; */

951: if (!pszSuff)

952: pszSuff = RTStrDup(".webm");

......

991: RTStrFree(pszSuff);

---

Let me try to explain why the error happen if the VM is just called like "abc.def" or "xxx.xxx":

Here we suggest the video recording path is "C:\abc.def.webm"(it's auto generated by VirtualBox GUI)

When the applicatioin comes to line 950:

pszAbsPath:C:\abc.def

After line 950:

pszSuff:.def

Yeah!!!We catch the bug.Got it?

So when the applicatioin comes to line 951:

the line 952 will not excute because the string pszSuff(".def") comes from RTPathSuffix(), which returns a string that cannot be release by RTStrFree().

So when the applicatioin comes to line 991, as we all know, it will definitely leads to a heap failure.

A solution is to modify the line 950 as I do above, such as "char *pszSuff = NULL". After that, the line 952 will execute and the string pointerd by pszSuff will comes from a string that can be released by RTStrFree() which return from RTStrDup().

---

Finally, I am just a nameless cleanner from china, thanks for updating the new version for the Shanghai/Zhaoxin CPU, thhough which are not developed but just copied by chinese companies. I hopes that in the fure, several Chinese companies will build a excellent PC/Server field's CPU which are wholly developped by Chinese engineers, even better than CPUs made by Intel/AMD.

[cleanner]

Excuse me, it is my first time to use the ticket system, here is some new idea about the bug:

As I mentioned above, a solution of this bug is to modify the line 950 to "char *pszSuff = NULL;".

However, it's still not perfect, because I found that if the video recording path is "C:\abc.def"(manually modified by user), it will still work incorrectly, it leads to a final path "C:\abc.webm" but not "C:\abc.def.webm", which including the VM full name "abc.def".

I think the main reason is the line 947.

It tries to remove the suffix from the video recording path unconditionally.

So let's just try the code below:

---

944 char *pszAbsPath = RTPathAbsDup(com::Utf8Str(pCfg->File.strName).c_str());

945 AssertPtrReturn(pszAbsPath, VERR_NO_MEMORY);

947 if (_stricmp(RTPathSuffix(pszAbsPath), ".webm") == 0)

948 {

949 RTPathStripSuffix(pszAbsPath);

950 }

952 AssertPtrReturn(pszAbsPath, VERR_INVALID_PARAMETER);

953 char *pszSuff = RTStrDup(".webm");

955 if (!pszSuff)

956 {

957 RTStrFree(pszAbsPath);

958 return VERR_NO_MEMORY;

959 }

---

Thanks!

Replying to cleanner:

When I set the VM with a speical name called "abc.def", it leads to a heap failure after I start the video recording.

It's because it finally free a string which can not be released by RTStrFree(), which will lead to a heap failure.

I found that the error comes from "src\VBox\Main\src-client\VideoRec.cpp", line 950.

---

944: char *pszAbsPath = RTPathAbsDup(com::Utf8Str(pCfg->File.strName).c_str());

945: AssertPtrReturn(pszAbsPath, VERR_NO_MEMORY);

947: RTPathStripSuffix(pszAbsPath);

948: AssertPtrReturn(pszAbsPath, VERR_INVALID_PARAMETER);

950: char *pszSuff = RTPathSuffix(pszAbsPath); /* it should be "char *pszSuff = NULL"; */

951: if (!pszSuff)

952: pszSuff = RTStrDup(".webm");

......

991: RTStrFree(pszSuff);

---

Let me try to explain why the error happen if the VM is just called like "abc.def" or "xxx.xxx":

Here we suggest the video recording path is "C:\abc.def.webm"(it's auto generated by VirtualBox GUI)

When the applicatioin comes to line 950:

pszAbsPath:C:\abc.def

After line 950:

pszSuff:.def

Yeah!!!We catch the bug.Got it?

So when the applicatioin comes to line 951:

the line 952 will not excute because the string pszSuff(".def") comes from RTPathSuffix(), which returns a string that cannot be release by RTStrFree().

So when the applicatioin comes to line 991, as we all know, it will definitely leads to a heap failure.

A solution is to modify the line 950 as I do above, such as "char *pszSuff = NULL". After that, the line 952 will execute and the string pointerd by pszSuff will comes from a string that can be released by RTStrFree() which return from RTStrDup().

---

Finally, I am just a nameless cleanner from china, thanks for updating the new version for the Shanghai/Zhaoxin CPU, thhough which are not developed but just copied by chinese companies. I hopes that in the fure, several Chinese companies will build a excellent PC/Server field's CPU which are wholly developped by Chinese engineers, even better than CPUs made by Intel/AMD.

[cleanner]

Sorry, I have checked the VirtualBox 5.2.26, and I found that you have fix the error I mentioned, however I think the second solution I mentioned above is better than the solution mention in VirtualBox 5.2.26.

I started to use 5.2.8 since a year ago, I just checked the newer version just right now.

take it: if (_stricmp(RTPathSuffix(pszAbsPath), ".webm") == 0)