Intravision/reactos
1
0
Fork 0
mirror of https://github.com/reactos/reactos.git synced 2025-07-31 19:52:11 +00:00
Code Issues Projects Releases Packages Wiki Activity

[ADVAPI32/SERVICES]

Browse source 
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.

svn path=/trunk/; revision=57328
This commit is contained in:
Eric Kohl 2012-09-18 21:54:43 +00:00
parent 0f13b36a15
commit 95b260a636
6 changed files with 92 additions and 75 deletions
Download patch file Download diff file Expand all files Collapse all files

62
reactos/base/system/services/config.c
View file

@ -246,19 +246,17 @@ ScmIsDeleteFlagSet(HKEY hServiceKey)
DWORD
ScmReadString(HKEY hServiceKey,
LPWSTR lpValueName,
LPCWSTR lpValueName,
LPWSTR *lpValue)
{
DWORD dwError;
DWORD dwSize;
DWORD dwType;
DWORD dwSizeNeeded;
LPWSTR expanded = NULL;
DWORD dwError = 0;
DWORD dwSize = 0;
DWORD dwType = 0;
LPWSTR ptr = NULL;
LPWSTR expanded = NULL;
*lpValue = NULL;
dwSize = 0;
dwError = RegQueryValueExW(hServiceKey,
lpValueName,
0,
@ -268,7 +266,7 @@ ScmReadString(HKEY hServiceKey,
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,40 +277,48 @@ ScmReadString(HKEY hServiceKey,
(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;
}

2
reactos/base/system/services/database.c
View file

@ -591,7 +591,7 @@ ScmDeleteRegKey(HKEY hKey, LPCWSTR lpszSubKey)
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)

46
reactos/base/system/services/rpcserver.c
View file

@ -301,7 +301,7 @@ ScmAssignNewTag(PSERVICE lpService)
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 @@ DWORD RSetServiceStatus(
}
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 @@ DWORD RChangeServiceConfigW(
/* Update the display name */
lpDisplayNameW = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
(wcslen(lpDisplayName) + 1) * sizeof(WCHAR));
if (lpDisplayNameW == NULL)
{
@ -2142,7 +2137,8 @@ DWORD RCreateServiceW(
*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 @@ DWORD REnumDependentServicesW(
/* Allocate memory for array of service pointers */
lpServicesArray = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
(dwServicesReturned + 1) * sizeof(PSERVICE));
if (!lpServicesArray)
{
@ -2447,7 +2443,7 @@ DWORD REnumDependentServicesW(
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 @@ DWORD REnumDependentServicesW(
lpServicesPtr->lpServiceName = (LPWSTR)((ULONG_PTR)lpStr - (ULONG_PTR)lpServices);
lpStr += (wcslen(lpService->lpServiceName) + 1);
lpServicesPtr ++;
lpServicesPtr++;
}
*lpServicesReturned = dwServicesReturned;
@ -3190,7 +3186,7 @@ DWORD RChangeServiceConfigA(
{
/* Set the display name */
lpDisplayNameW = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
(strlen(lpDisplayName) + 1) * sizeof(WCHAR));
if (lpDisplayNameW == NULL)
{
@ -3268,7 +3264,7 @@ DWORD RChangeServiceConfigA(
{
/* Set the image path */
lpBinaryPathNameW = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
(strlen(lpBinaryPathName) + 1) * sizeof(WCHAR));
if (lpBinaryPathNameW == NULL)
{
@ -3314,7 +3310,7 @@ DWORD RChangeServiceConfigA(
if (lpLoadOrderGroup != NULL && *lpLoadOrderGroup != 0)
{
lpLoadOrderGroupW = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
(strlen(lpLoadOrderGroup) + 1) * sizeof(WCHAR));
if (lpLoadOrderGroupW == NULL)
{
@ -3372,7 +3368,7 @@ DWORD RChangeServiceConfigA(
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 @@ DWORD RCreateServiceA(
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 @@ DWORD RCreateServiceA(
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 @@ DWORD RCreateServiceA(
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 @@ DWORD RCreateServiceA(
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 @@ DWORD RCreateServiceA(
}
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 @@ DWORD RCreateServiceA(
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 @@ DWORD REnumDependentServicesA(
/* Allocate memory for array of service pointers */
lpServicesArray = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
(dwServicesReturned + 1) * sizeof(PSERVICE));
if (!lpServicesArray)
{
@ -3698,7 +3694,7 @@ DWORD REnumDependentServicesA(
lpServicesPtr->lpServiceName = (LPSTR)((ULONG_PTR)lpStr - (ULONG_PTR)lpServices);
lpStr += strlen(lpStr) + 1;
lpServicesPtr ++;
lpServicesPtr++;
}
*lpServicesReturned = dwServicesReturned;
@ -4755,7 +4751,7 @@ DWORD RChangeServiceConfig2A(
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 @@ DWORD RChangeServiceConfig2A(
dwLength = dwRebootLen + dwCommandLen + sizeof(SERVICE_FAILURE_ACTIONSW);
lpServiceFailureActionsW = HeapAlloc(GetProcessHeap(),
0,
HEAP_ZERO_MEMORY,
dwLength);
if (!lpServiceFailureActionsW)
{

2
reactos/base/system/services/services.h
View file

@ -106,7 +106,7 @@ DWORD ScmMarkServiceForDelete(PSERVICE pService);
BOOL ScmIsDeleteFlagSet(HKEY hServiceKey);
DWORD ScmReadString(HKEY hServiceKey,
LPWSTR lpValueName,
LPCWSTR lpValueName,
LPWSTR *lpValue);
DWORD

40
reactos/dll/win32/advapi32/service/scm.c
View file

@ -287,7 +287,7 @@ ChangeServiceConfigA(SC_HANDLE hService,
{
DWORD dwError;
DWORD dwDependenciesLength = 0;
DWORD dwLength;
SIZE_T cchLength;
LPCSTR lpStr;
DWORD dwPasswordLength = 0;
LPBYTE lpEncryptedPassword = NULL;
@ -300,16 +300,16 @@ ChangeServiceConfigA(SC_HANDLE hService,
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 @@ ChangeServiceConfigW(SC_HANDLE hService,
{
DWORD dwError;
DWORD dwDependenciesLength = 0;
DWORD dwLength;
SIZE_T cchLength;
LPCWSTR lpStr;
DWORD dwPasswordLength = 0;
LPBYTE lpEncryptedPassword = NULL;
@ -378,11 +378,12 @@ ChangeServiceConfigW(SC_HANDLE hService,
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 @@ CreateServiceA(SC_HANDLE hSCManager,
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 @@ CreateServiceA(SC_HANDLE hSCManager,
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 @@ CreateServiceW(SC_HANDLE hSCManager,
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 @@ CreateServiceW(SC_HANDLE hSCManager,
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
{

15
reactos/dll/win32/advapi32/service/sctrl.c
View file

@ -290,6 +290,9 @@ ScBuildUnicodeArgsVector(PSCM_CONTROL_PACKET ControlPacket,
LPWSTR *lpArg;
DWORD i;
if (ControlPacket == NULL || lpArgCount == NULL || lpArgVector == NULL)
return ERROR_INVALID_PARAMETER;
*lpArgCount = 0;
*lpArgVector = NULL;
@ -334,6 +337,9 @@ ScBuildAnsiArgsVector(PSCM_CONTROL_PACKET ControlPacket,
DWORD dwAnsiSize;
DWORD i;
if (ControlPacket == NULL || lpArgCount == NULL || lpArgVector == NULL)
return ERROR_INVALID_PARAMETER;
*lpArgCount = 0;
*lpArgVector = NULL;
@ -399,6 +405,9 @@ ScStartService(PACTIVE_SERVICE lpService,
DWORD ThreadId;
DWORD dwError;
if (lpService == NULL || ControlPacket == NULL)
return ERROR_INVALID_PARAMETER;
TRACE("ScStartService() called\n");
TRACE("Size: %lu\n", ControlPacket->dwSize);
TRACE("Service: %S\n", (PWSTR)((PBYTE)ControlPacket + ControlPacket->dwServiceNameOffset));
@ -470,6 +479,9 @@ static DWORD
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));
@ -505,6 +517,9 @@ ScServiceDispatcher(HANDLE hPipe,
TRACE("ScDispatcherLoop() called\n");
if (ControlPacket == NULL || dwBufferSize < sizeof(SCM_CONTROL_PACKET))
return FALSE;
while (TRUE)
{
/* Read command from the control pipe */