Opened 6 years ago
Closed 6 years ago
#18513 closed defect (obsolete)
Video recording file path including dot char leads to a heap failure.
Reported by: | cleanner | Owned by: | |
---|---|---|---|
Component: | GUI | Version: | VirtualBox 5.2.8 |
Keywords: | Video recording heap failure | Cc: | |
Guest type: | all | Host type: | Windows |
Description
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.
Change History (4)
comment:3 by , 6 years ago
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)
comment:4 by , 6 years ago
Resolution: | → obsolete |
---|---|
Status: | new → closed |
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 }
It seems to be a better solution.