[ekohl] 57328: [ADVAPI32/SERVICES] This patch fixes various things, from Coverity code defects to conversion warnings : - CID 715948 (logically dead code @ services/rpcserver.c) - try to fix CID 71...
18 Sep
2012
18 Sep
'12
9:54 p.m.
Author: ekohl
Date: Tue Sep 18 21:54:43 2012
New Revision: 57328
URL: http://svn.reactos.org/svn/reactos?rev=57328&view=rev
Log:
[ADVAPI32/SERVICES]
This patch fixes various things, from Coverity code defects to conversion
warnings :
- CID 715948 (logically dead code @ services/rpcserver.c)
- try to fix CID 716332/3 (resource leaks) by rewriting the ScmReadString
function (@ services/config.c)
- zero out the freshly allocated memory (@ services)
- try to fix CID 716126/7/8 (untrusted value as argument @
advapi32/services/sctrl.c)
Fix also some "size_t to DWORD" warnings on x64 build (@
advapi32/services/scm.c).
Patch by Hermes BELUSCA - MAITO.
Fixes CORE-6606.
Modified:
trunk/reactos/base/system/services/config.c
trunk/reactos/base/system/services/database.c
trunk/reactos/base/system/services/rpcserver.c
trunk/reactos/base/system/services/services.h
trunk/reactos/dll/win32/advapi32/service/scm.c
trunk/reactos/dll/win32/advapi32/service/sctrl.c
Modified: trunk/reactos/base/system/services/config.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/confi…
==============================================================================
--- trunk/reactos/base/system/services/config.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/config.c [iso-8859-1] Tue Sep 18 21:54:43 2012
@@ -246,19 +246,17 @@
DWORD
ScmReadString(HKEY hServiceKey,
- LPWSTR lpValueName,
+ LPCWSTR lpValueName,
LPWSTR *lpValue)
{
- DWORD dwError;
- DWORD dwSize;
- DWORD dwType;
- DWORD dwSizeNeeded;
+ DWORD dwError = 0;
+ DWORD dwSize = 0;
+ DWORD dwType = 0;
+ LPWSTR ptr = NULL;
LPWSTR expanded = NULL;
- LPWSTR ptr = NULL;
*lpValue = NULL;
- dwSize = 0;
dwError = RegQueryValueExW(hServiceKey,
lpValueName,
0,
@@ -268,7 +266,7 @@
if (dwError != ERROR_SUCCESS)
return dwError;
- ptr = HeapAlloc(GetProcessHeap(), 0, dwSize);
+ ptr = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dwSize);
if (ptr == NULL)
return ERROR_NOT_ENOUGH_MEMORY;
@@ -279,38 +277,46 @@
(LPBYTE)ptr,
&dwSize);
if (dwError != ERROR_SUCCESS)
- goto done;
+ {
+ HeapFree(GetProcessHeap(), 0, ptr);
+ return dwError;
+ }
if (dwType == REG_EXPAND_SZ)
{
/* Expand the value... */
- dwSizeNeeded = ExpandEnvironmentStringsW((LPCWSTR)ptr, NULL, 0);
- if (dwSizeNeeded == 0)
+ dwSize = ExpandEnvironmentStringsW(ptr, NULL, 0);
+ if (dwSize > 0)
+ {
+ expanded = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dwSize *
sizeof(WCHAR));
+ if (expanded)
+ {
+ if (dwSize == ExpandEnvironmentStringsW(ptr, expanded, dwSize))
+ {
+ *lpValue = expanded;
+ dwError = ERROR_SUCCESS;
+ }
+ else
+ {
+ dwError = GetLastError();
+ HeapFree(GetProcessHeap(), 0, expanded);
+ }
+ }
+ else
+ {
+ dwError = ERROR_NOT_ENOUGH_MEMORY;
+ }
+ }
+ else
{
dwError = GetLastError();
- goto done;
- }
- expanded = HeapAlloc(GetProcessHeap(), 0, dwSizeNeeded * sizeof(WCHAR));
- if (dwSizeNeeded < ExpandEnvironmentStringsW((LPCWSTR)ptr, expanded,
dwSizeNeeded))
- {
- dwError = GetLastError();
- goto done;
- }
- *lpValue = expanded;
+ }
+
HeapFree(GetProcessHeap(), 0, ptr);
- dwError = ERROR_SUCCESS;
}
else
{
*lpValue = ptr;
- }
-
-done:
- if (dwError != ERROR_SUCCESS)
- {
- HeapFree(GetProcessHeap(), 0, ptr);
- if (expanded)
- HeapFree(GetProcessHeap(), 0, expanded);
}
return dwError;
Modified: trunk/reactos/base/system/services/database.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/datab…
==============================================================================
--- trunk/reactos/base/system/services/database.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/database.c [iso-8859-1] Tue Sep 18 21:54:43 2012
@@ -591,7 +591,7 @@
if (dwMaxSubkeyLen > sizeof(szNameBuf) / sizeof(WCHAR))
{
/* Name too big: alloc a buffer for it */
- lpszName = HeapAlloc(GetProcessHeap(), 0, dwMaxSubkeyLen *
sizeof(WCHAR));
+ lpszName = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dwMaxSubkeyLen *
sizeof(WCHAR));
}
if (!lpszName)
Modified: trunk/reactos/base/system/services/rpcserver.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/rpcse…
==============================================================================
--- trunk/reactos/base/system/services/rpcserver.c [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/rpcserver.c [iso-8859-1] Tue Sep 18 21:54:43 2012
@@ -301,7 +301,7 @@
if (dwError != ERROR_SUCCESS && dwError != ERROR_MORE_DATA)
goto findFreeTag;
- pdwGroupTags = HeapAlloc(GetProcessHeap(), 0, cbDataSize);
+ pdwGroupTags = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, cbDataSize);
if (!pdwGroupTags)
{
dwError = ERROR_NOT_ENOUGH_MEMORY;
@@ -1635,11 +1635,6 @@
}
lpService = (PSERVICE)hServiceStatus;
- if (lpService == NULL)
- {
- DPRINT("lpService == NULL!\n");
- return ERROR_INVALID_HANDLE;
- }
/* Check current state */
if (!ScmIsValidServiceState(lpServiceStatus->dwCurrentState))
@@ -1819,7 +1814,7 @@
/* Update the display name */
lpDisplayNameW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(wcslen(lpDisplayName) + 1) * sizeof(WCHAR));
if (lpDisplayNameW == NULL)
{
@@ -2142,7 +2137,8 @@
*lpDisplayName != 0 &&
_wcsicmp(lpService->lpDisplayName, lpDisplayName) != 0)
{
- lpService->lpDisplayName = HeapAlloc(GetProcessHeap(), 0,
+ lpService->lpDisplayName = HeapAlloc(GetProcessHeap(),
+ HEAP_ZERO_MEMORY,
(wcslen(lpDisplayName) + 1) *
sizeof(WCHAR));
if (lpService->lpDisplayName == NULL)
{
@@ -2424,7 +2420,7 @@
/* Allocate memory for array of service pointers */
lpServicesArray = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(dwServicesReturned + 1) * sizeof(PSERVICE));
if (!lpServicesArray)
{
@@ -2447,7 +2443,7 @@
goto Done;
}
- lpServicesPtr = (LPENUM_SERVICE_STATUSW) lpServices;
+ lpServicesPtr = (LPENUM_SERVICE_STATUSW)lpServices;
lpStr = (LPWSTR)(lpServices + (dwServicesReturned * sizeof(ENUM_SERVICE_STATUSW)));
/* Copy EnumDepenedentService to Buffer */
@@ -2470,7 +2466,7 @@
lpServicesPtr->lpServiceName = (LPWSTR)((ULONG_PTR)lpStr -
(ULONG_PTR)lpServices);
lpStr += (wcslen(lpService->lpServiceName) + 1);
- lpServicesPtr ++;
+ lpServicesPtr++;
}
*lpServicesReturned = dwServicesReturned;
@@ -3190,7 +3186,7 @@
{
/* Set the display name */
lpDisplayNameW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(strlen(lpDisplayName) + 1) * sizeof(WCHAR));
if (lpDisplayNameW == NULL)
{
@@ -3268,7 +3264,7 @@
{
/* Set the image path */
lpBinaryPathNameW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(strlen(lpBinaryPathName) + 1) * sizeof(WCHAR));
if (lpBinaryPathNameW == NULL)
{
@@ -3314,7 +3310,7 @@
if (lpLoadOrderGroup != NULL && *lpLoadOrderGroup != 0)
{
lpLoadOrderGroupW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(strlen(lpLoadOrderGroup) + 1) * sizeof(WCHAR));
if (lpLoadOrderGroupW == NULL)
{
@@ -3372,7 +3368,7 @@
if (lpDependencies != NULL && *lpDependencies != 0)
{
lpDependenciesW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(strlen((LPSTR)lpDependencies) + 1) *
sizeof(WCHAR));
if (lpDependenciesW == NULL)
{
@@ -3446,7 +3442,7 @@
if (lpServiceName)
{
len = MultiByteToWideChar(CP_ACP, 0, lpServiceName, -1, NULL, 0);
- lpServiceNameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+ lpServiceNameW = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len *
sizeof(WCHAR));
if (!lpServiceNameW)
{
SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3458,7 +3454,7 @@
if (lpDisplayName)
{
len = MultiByteToWideChar(CP_ACP, 0, lpDisplayName, -1, NULL, 0);
- lpDisplayNameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+ lpDisplayNameW = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len *
sizeof(WCHAR));
if (!lpDisplayNameW)
{
SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3470,7 +3466,7 @@
if (lpBinaryPathName)
{
len = MultiByteToWideChar(CP_ACP, 0, lpBinaryPathName, -1, NULL, 0);
- lpBinaryPathNameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+ lpBinaryPathNameW = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len *
sizeof(WCHAR));
if (!lpBinaryPathNameW)
{
SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3482,7 +3478,7 @@
if (lpLoadOrderGroup)
{
len = MultiByteToWideChar(CP_ACP, 0, lpLoadOrderGroup, -1, NULL, 0);
- lpLoadOrderGroupW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+ lpLoadOrderGroupW = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len *
sizeof(WCHAR));
if (!lpLoadOrderGroupW)
{
SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3502,7 +3498,7 @@
}
dwDependenciesLength++;
- lpDependenciesW = HeapAlloc(GetProcessHeap(), 0, dwDependenciesLength *
sizeof(WCHAR));
+ lpDependenciesW = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
dwDependenciesLength * sizeof(WCHAR));
if (!lpDependenciesW)
{
SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3514,7 +3510,7 @@
if (lpServiceStartName)
{
len = MultiByteToWideChar(CP_ACP, 0, lpServiceStartName, -1, NULL, 0);
- lpServiceStartNameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+ lpServiceStartNameW = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, len *
sizeof(WCHAR));
if (!lpServiceStartNameW)
{
SetLastError(ERROR_NOT_ENOUGH_MEMORY);
@@ -3638,7 +3634,7 @@
/* Allocate memory for array of service pointers */
lpServicesArray = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
(dwServicesReturned + 1) * sizeof(PSERVICE));
if (!lpServicesArray)
{
@@ -3698,7 +3694,7 @@
lpServicesPtr->lpServiceName = (LPSTR)((ULONG_PTR)lpStr -
(ULONG_PTR)lpServices);
lpStr += strlen(lpStr) + 1;
- lpServicesPtr ++;
+ lpServicesPtr++;
}
*lpServicesReturned = dwServicesReturned;
@@ -4755,7 +4751,7 @@
dwLength = (DWORD)((strlen(Info.lpDescription) + 1) * sizeof(WCHAR));
lpServiceDescriptonW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
dwLength + sizeof(SERVICE_DESCRIPTIONW));
if (!lpServiceDescriptonW)
{
@@ -4797,7 +4793,7 @@
dwLength = dwRebootLen + dwCommandLen + sizeof(SERVICE_FAILURE_ACTIONSW);
lpServiceFailureActionsW = HeapAlloc(GetProcessHeap(),
- 0,
+ HEAP_ZERO_MEMORY,
dwLength);
if (!lpServiceFailureActionsW)
{
Modified: trunk/reactos/base/system/services/services.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/servi…
==============================================================================
--- trunk/reactos/base/system/services/services.h [iso-8859-1] (original)
+++ trunk/reactos/base/system/services/services.h [iso-8859-1] Tue Sep 18 21:54:43 2012
@@ -106,7 +106,7 @@
BOOL ScmIsDeleteFlagSet(HKEY hServiceKey);
DWORD ScmReadString(HKEY hServiceKey,
- LPWSTR lpValueName,
+ LPCWSTR lpValueName,
LPWSTR *lpValue);
DWORD
Modified: trunk/reactos/dll/win32/advapi32/service/scm.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/service…
==============================================================================
--- trunk/reactos/dll/win32/advapi32/service/scm.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/advapi32/service/scm.c [iso-8859-1] Tue Sep 18 21:54:43 2012
@@ -287,7 +287,7 @@
{
DWORD dwError;
DWORD dwDependenciesLength = 0;
- DWORD dwLength;
+ SIZE_T cchLength;
LPCSTR lpStr;
DWORD dwPasswordLength = 0;
LPBYTE lpEncryptedPassword = NULL;
@@ -300,16 +300,16 @@
lpStr = lpDependencies;
while (*lpStr)
{
- dwLength = strlen(lpStr) + 1;
- dwDependenciesLength += dwLength;
- lpStr = lpStr + dwLength;
+ cchLength = strlen(lpStr) + 1;
+ dwDependenciesLength += (DWORD)cchLength;
+ lpStr = lpStr + cchLength;
}
dwDependenciesLength++;
}
/* FIXME: Encrypt the password */
lpEncryptedPassword = (LPBYTE)lpPassword;
- dwPasswordLength = (lpPassword ? (strlen(lpPassword) + 1) * sizeof(CHAR) : 0);
+ dwPasswordLength = (DWORD)(lpPassword ? (strlen(lpPassword) + 1) * sizeof(CHAR) :
0);
RpcTryExcept
{
@@ -365,7 +365,7 @@
{
DWORD dwError;
DWORD dwDependenciesLength = 0;
- DWORD dwLength;
+ SIZE_T cchLength;
LPCWSTR lpStr;
DWORD dwPasswordLength = 0;
LPBYTE lpEncryptedPassword = NULL;
@@ -378,11 +378,12 @@
lpStr = lpDependencies;
while (*lpStr)
{
- dwLength = wcslen(lpStr) + 1;
- dwDependenciesLength += dwLength;
- lpStr = lpStr + dwLength;
+ cchLength = wcslen(lpStr) + 1;
+ dwDependenciesLength += (DWORD)cchLength;
+ lpStr = lpStr + cchLength;
}
dwDependenciesLength++;
+ dwDependenciesLength *= sizeof(WCHAR);
}
/* FIXME: Encrypt the password */
@@ -547,7 +548,7 @@
SC_HANDLE hService = NULL;
DWORD dwDependenciesLength = 0;
DWORD dwError;
- DWORD dwLength;
+ SIZE_T cchLength;
LPCSTR lpStr;
DWORD dwPasswordLength = 0;
LPBYTE lpEncryptedPassword = NULL;
@@ -568,16 +569,16 @@
lpStr = lpDependencies;
while (*lpStr)
{
- dwLength = strlen(lpStr) + 1;
- dwDependenciesLength += dwLength;
- lpStr = lpStr + dwLength;
+ cchLength = strlen(lpStr) + 1;
+ dwDependenciesLength += (DWORD)cchLength;
+ lpStr = lpStr + cchLength;
}
dwDependenciesLength++;
}
/* FIXME: Encrypt the password */
lpEncryptedPassword = (LPBYTE)lpPassword;
- dwPasswordLength = (lpPassword ? (strlen(lpPassword) + 1) * sizeof(CHAR) : 0);
+ dwPasswordLength = (DWORD)(lpPassword ? (strlen(lpPassword) + 1) * sizeof(CHAR) :
0);
RpcTryExcept
{
@@ -639,7 +640,7 @@
SC_HANDLE hService = NULL;
DWORD dwDependenciesLength = 0;
DWORD dwError;
- DWORD dwLength;
+ SIZE_T cchLength;
LPCWSTR lpStr;
DWORD dwPasswordLength = 0;
LPBYTE lpEncryptedPassword = NULL;
@@ -660,18 +661,17 @@
lpStr = lpDependencies;
while (*lpStr)
{
- dwLength = wcslen(lpStr) + 1;
- dwDependenciesLength += dwLength;
- lpStr = lpStr + dwLength;
+ cchLength = wcslen(lpStr) + 1;
+ dwDependenciesLength += (DWORD)cchLength;
+ lpStr = lpStr + cchLength;
}
dwDependenciesLength++;
-
dwDependenciesLength *= sizeof(WCHAR);
}
/* FIXME: Encrypt the password */
lpEncryptedPassword = (LPBYTE)lpPassword;
- dwPasswordLength = (lpPassword ? (wcslen(lpPassword) + 1) * sizeof(WCHAR) : 0);
+ dwPasswordLength = (DWORD)(lpPassword ? (wcslen(lpPassword) + 1) * sizeof(WCHAR) :
0);
RpcTryExcept
{
Modified: trunk/reactos/dll/win32/advapi32/service/sctrl.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/service…
==============================================================================
--- trunk/reactos/dll/win32/advapi32/service/sctrl.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/advapi32/service/sctrl.c [iso-8859-1] Tue Sep 18 21:54:43
2012
@@ -290,6 +290,9 @@
LPWSTR *lpArg;
DWORD i;
+ if (ControlPacket == NULL || lpArgCount == NULL || lpArgVector == NULL)
+ return ERROR_INVALID_PARAMETER;
+
*lpArgCount = 0;
*lpArgVector = NULL;
@@ -334,6 +337,9 @@
DWORD dwAnsiSize;
DWORD i;
+ if (ControlPacket == NULL || lpArgCount == NULL || lpArgVector == NULL)
+ return ERROR_INVALID_PARAMETER;
+
*lpArgCount = 0;
*lpArgVector = NULL;
@@ -398,6 +404,9 @@
HANDLE ThreadHandle;
DWORD ThreadId;
DWORD dwError;
+
+ if (lpService == NULL || ControlPacket == NULL)
+ return ERROR_INVALID_PARAMETER;
TRACE("ScStartService() called\n");
TRACE("Size: %lu\n", ControlPacket->dwSize);
@@ -470,6 +479,9 @@
ScControlService(PACTIVE_SERVICE lpService,
PSCM_CONTROL_PACKET ControlPacket)
{
+ if (lpService == NULL || ControlPacket == NULL)
+ return ERROR_INVALID_PARAMETER;
+
TRACE("ScControlService() called\n");
TRACE("Size: %lu\n", ControlPacket->dwSize);
TRACE("Service: %S\n", (PWSTR)((PBYTE)ControlPacket +
ControlPacket->dwServiceNameOffset));
@@ -504,6 +516,9 @@
DWORD dwError;
TRACE("ScDispatcherLoop() called\n");
+
+ if (ControlPacket == NULL || dwBufferSize < sizeof(SCM_CONTROL_PACKET))
+ return FALSE;
while (TRUE)
{
4439
days inactive
4439
days old
0 comments
1 participants
participants (1)
-
ekohl＠svn.reactos.org