19 Dec
2010
19 Dec
'10
11:19 p.m.
Author: rmessiant
Date: Sun Dec 19 23:19:04 2010
New Revision: 50066
URL: http://svn.reactos.org/svn/reactos?rev=50066&view=rev
Log:
[ADVAPI32]
- ConvertStringSidToSidW: Stop writing 1 subauthority too much. Fixes a DWORD sized buffer
overflow. Should fix bug #5764.
- ConvertStringSidToSidW: Don't leak an allocated SID in case of failure.
[SETUPAPI]
- SetupDiClassNameFromGuidExW: Rewrite to prevent a buffer overflow and pass additional
winetests. Should fix bug #5474.
- SetupDiClassNameFromGuidExA: Return the required buffer size in failure cases.
Modified:
trunk/reactos/dll/win32/advapi32/sec/sid.c
trunk/reactos/dll/win32/setupapi/devinst.c
Modified: trunk/reactos/dll/win32/advapi32/sec/sid.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/sec/sid…
==============================================================================
--- trunk/reactos/dll/win32/advapi32/sec/sid.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/advapi32/sec/sid.c [iso-8859-1] Sun Dec 19 23:19:04 2010
@@ -1982,16 +1982,14 @@
if (*StringSid == '-')
StringSid++;
- pisid->SubAuthority[i] = atoiW(StringSid);
-
while (*StringSid)
{
+ pisid->SubAuthority[i++] = atoiW(StringSid);
+
while (*StringSid && *StringSid != '-')
StringSid++;
if (*StringSid == '-')
StringSid++;
-
- pisid->SubAuthority[++i] = atoiW(StringSid);
}
if (i != pisid->SubAuthorityCount)
@@ -2002,7 +2000,10 @@
lend:
if (!ret)
+ {
+ LocalFree(pisid);
SetLastError(ERROR_INVALID_SID);
+ }
TRACE("returning %s\n", ret ? "TRUE" : "FALSE");
return ret;
Modified: trunk/reactos/dll/win32/setupapi/devinst.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/setupapi/devinst…
==============================================================================
--- trunk/reactos/dll/win32/setupapi/devinst.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/setupapi/devinst.c [iso-8859-1] Sun Dec 19 23:19:04 2010
@@ -1103,7 +1103,7 @@
if (MachineName)
MachineNameW = MultiByteToUnicode(MachineName, CP_ACP);
ret = SetupDiClassNameFromGuidExW(ClassGuid, ClassNameW, MAX_CLASS_NAME_LEN,
- NULL, MachineNameW, Reserved);
+ RequiredSize, MachineNameW, Reserved);
if (ret)
{
int len = WideCharToMultiByte(CP_ACP, 0, ClassNameW, -1, ClassName,
@@ -1113,8 +1113,6 @@
SetLastError(ERROR_INSUFFICIENT_BUFFER);
ret = FALSE;
}
- else if (RequiredSize)
- *RequiredSize = len;
}
MyFree(MachineNameW);
return ret;
@@ -1135,73 +1133,98 @@
DWORD dwLength;
DWORD dwRegType;
LONG rc;
+ PWSTR Buffer;
TRACE("%s %p %lu %p %s %p\n", debugstr_guid(ClassGuid), ClassName,
ClassNameSize, RequiredSize, debugstr_w(MachineName), Reserved);
- hKey = SetupDiOpenClassRegKeyExW(ClassGuid,
- KEY_QUERY_VALUE,
- DIOCR_INSTALLER,
- MachineName,
- Reserved);
+ /* Make sure there's a GUID */
+ if (ClassGuid == NULL)
+ {
+ SetLastError(ERROR_INVALID_CLASS); /* On Vista: ERROR_INVALID_USER_BUFFER */
+ return FALSE;
+ }
+
+ /* Make sure there's a real buffer when there's a size */
+ if ((ClassNameSize > 0) && (ClassName == NULL))
+ {
+ SetLastError(ERROR_INVALID_PARAMETER); /* On Vista: ERROR_INVALID_USER_BUFFER
*/
+ return FALSE;
+ }
+
+ /* Open the key for the GUID */
+ hKey = SetupDiOpenClassRegKeyExW(ClassGuid, KEY_QUERY_VALUE, DIOCR_INSTALLER,
MachineName, Reserved);
+
if (hKey == INVALID_HANDLE_VALUE)
- {
- return FALSE;
- }
-
+ return FALSE;
+
+ /* Retrieve the class name data */
+ dwLength = ClassNameSize * sizeof(WCHAR);
+
+ do
+ {
+ /* Allocate a buffer to retrieve the class name data */
+ Buffer = HeapAlloc(GetProcessHeap(), 0, dwLength);
+
+ if (Buffer == NULL)
+ {
+ rc = GetLastError();
+ break;
+ }
+
+ /* Query for the class name data */
+ rc = RegQueryValueExW(hKey, Class, NULL, &dwRegType, (LPBYTE) Buffer,
&dwLength);
+
+ /* Clean up the buffer if needed */
+ if (rc != ERROR_SUCCESS)
+ HeapFree(GetProcessHeap(), 0, Buffer);
+ } while (rc == ERROR_MORE_DATA);
+
+ /* Close the key */
+ RegCloseKey(hKey);
+
+ /* Make sure we got the data */
+ if (rc != ERROR_SUCCESS)
+ {
+ SetLastError(rc);
+ return FALSE;
+ }
+
+ /* Make sure the data is a string */
+ if (dwRegType != REG_SZ)
+ {
+ HeapFree(GetProcessHeap(), 0, Buffer);
+ SetLastError(ERROR_GEN_FAILURE);
+ return FALSE;
+ }
+
+ /* Determine the length of the class name */
+ dwLength /= sizeof(WCHAR);
+
+ if ((dwLength == 0) || (Buffer[dwLength - 1] != UNICODE_NULL))
+ /* Count the null-terminator */
+ dwLength++;
+
+ /* Inform the caller about the class name */
+ if ((ClassName != NULL) && (dwLength <= ClassNameSize))
+ {
+ memcpy(ClassName, Buffer, (dwLength - 1) * sizeof(WCHAR));
+ ClassName[dwLength - 1] = UNICODE_NULL;
+ }
+
+ /* Inform the caller about the required size */
if (RequiredSize != NULL)
- {
- dwLength = 0;
- if (RegQueryValueExW(hKey,
- Class,
- NULL,
- NULL,
- NULL,
- &dwLength))
- {
- RegCloseKey(hKey);
- return FALSE;
- }
-
- *RequiredSize = dwLength / sizeof(WCHAR) + 1;
- }
-
- if (!ClassGuid)
- {
- SetLastError(ERROR_INVALID_CLASS);
- RegCloseKey(hKey);
- return FALSE;
- }
- if (!ClassName && ClassNameSize > 0)
- {
- SetLastError(ERROR_INVALID_PARAMETER);
- RegCloseKey(hKey);
- return FALSE;
- }
-
- dwLength = ClassNameSize * sizeof(WCHAR) - sizeof(UNICODE_NULL);
- rc = RegQueryValueExW(hKey,
- Class,
- NULL,
- &dwRegType,
- (LPBYTE)ClassName,
- &dwLength);
- if (rc != ERROR_SUCCESS)
- {
- SetLastError(rc);
- RegCloseKey(hKey);
- return FALSE;
- }
- if (dwRegType != REG_SZ)
- {
- SetLastError(ERROR_GEN_FAILURE);
- RegCloseKey(hKey);
- return FALSE;
- }
-
- if (ClassNameSize > 1)
- ClassName[ClassNameSize] = UNICODE_NULL;
- RegCloseKey(hKey);
+ *RequiredSize = dwLength;
+
+ /* Clean up the buffer */
+ HeapFree(GetProcessHeap(), 0, Buffer);
+
+ /* Make sure the buffer was large enough */
+ if ((ClassName == NULL) || (dwLength > ClassNameSize))
+ {
+ SetLastError(ERROR_INSUFFICIENT_BUFFER);
+ return FALSE;
+ }
return TRUE;
}