[tfaber] 59360: [SVCHOST] - Fix various incorrect buffer size calculations - Don't expand environment variables in REG_SZ values - Repair all DPRINTs - Other misc fixes CORE-7296 #resolve
Author: tfaber Date: Sat Jun 29 13:01:35 2013 New Revision: 59360
URL: http://svn.reactos.org/svn/reactos?rev=59360&view=rev Log: [SVCHOST] - Fix various incorrect buffer size calculations - Don't expand environment variables in REG_SZ values - Repair all DPRINTs - Other misc fixes CORE-7296 #resolve
Modified: trunk/reactos/base/services/svchost/svchost.c trunk/reactos/base/services/svchost/svchost.h
Modified: trunk/reactos/base/services/svchost/svchost.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/svchost/svcho... ============================================================================== --- trunk/reactos/base/services/svchost/svchost.c [iso-8859-1] (original) +++ trunk/reactos/base/services/svchost/svchost.c [iso-8859-1] Sat Jun 29 13:01:35 2013 @@ -19,7 +19,6 @@ static PCWSTR SERVICE_KEY = L"SYSTEM\CurrentControlSet\Services\"; static PCWSTR PARAMETERS_KEY = L"\Parameters";
-#define SERVICE_KEY_LENGTH wcslen(SERVICE_KEY); #define REG_MAX_DATA_SIZE 2048
static PSERVICE FirstService = NULL; @@ -29,104 +28,143 @@ BOOL PrepareService(PCWSTR ServiceName) { HKEY hServiceKey; - WCHAR ServiceKeyBuffer[MAX_PATH + 1]; - DWORD LeftOfBuffer = sizeof(ServiceKeyBuffer) / sizeof(ServiceKeyBuffer[0]); - DWORD KeyType; - PWSTR Buffer = NULL; - DWORD BufferSize = MAX_PATH + 1; + WCHAR ServiceKeyBuffer[512]; + HRESULT Result; + DWORD ValueType; + PWSTR Buffer; + DWORD BufferSize; LONG RetVal; HINSTANCE hServiceDll; - WCHAR DllPath[MAX_PATH + 2]; /* See MSDN on ExpandEnvironmentStrings() for ANSI strings for more details on + 2 */ + PWSTR DllPath; + DWORD DllPathSize; LPSERVICE_MAIN_FUNCTION ServiceMainFunc; PSERVICE Service; + DWORD ServiceNameSize;
/* Compose the registry path to the service's "Parameter" key */ - wcsncpy(ServiceKeyBuffer, SERVICE_KEY, LeftOfBuffer); - LeftOfBuffer -= wcslen(SERVICE_KEY); - wcsncat(ServiceKeyBuffer, ServiceName, LeftOfBuffer); - LeftOfBuffer -= wcslen(ServiceName); - wcsncat(ServiceKeyBuffer, PARAMETERS_KEY, LeftOfBuffer); - LeftOfBuffer -= wcslen(PARAMETERS_KEY); - - if (LeftOfBuffer < 0) - { - DPRINT1("Buffer overflow for service name: '%s'\n", ServiceName); + Result = StringCbCopy(ServiceKeyBuffer, sizeof(ServiceKeyBuffer), SERVICE_KEY); + if (FAILED(Result)) + { + DPRINT1("Buffer overflow for service name: '%ls'\n", ServiceName); + return FALSE; + } + Result = StringCbCat(ServiceKeyBuffer, sizeof(ServiceKeyBuffer), ServiceName); + if (FAILED(Result)) + { + DPRINT1("Buffer overflow for service name: '%ls'\n", ServiceName); + return FALSE; + } + Result = StringCbCat(ServiceKeyBuffer, sizeof(ServiceKeyBuffer), PARAMETERS_KEY); + if (FAILED(Result)) + { + DPRINT1("Buffer overflow for service name: '%ls'\n", ServiceName); return FALSE; }
/* Open the service registry key to find the dll name */ - if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, ServiceKeyBuffer, 0, KEY_READ, &hServiceKey) != ERROR_SUCCESS) - { - DPRINT1("Could not open service key (%s)\n", ServiceKeyBuffer); - return FALSE; - } - - do - { - if (Buffer) + RetVal = RegOpenKeyEx(HKEY_LOCAL_MACHINE, ServiceKeyBuffer, 0, KEY_READ, &hServiceKey); + if (RetVal != ERROR_SUCCESS) + { + DPRINT1("Could not open service key (%ls)\n", ServiceKeyBuffer); + return FALSE; + } + + BufferSize = 0; + RetVal = RegQueryValueEx(hServiceKey, L"ServiceDll", NULL, &ValueType, NULL, &BufferSize); + if (RetVal != ERROR_SUCCESS) + { + RegCloseKey(hServiceKey); + return FALSE; + } + + Buffer = HeapAlloc(GetProcessHeap(), 0, BufferSize); + if (Buffer == NULL) + { + DPRINT1("Not enough memory for service: %ls\n", ServiceName); + RegCloseKey(hServiceKey); + return FALSE; + } + + RetVal = RegQueryValueEx(hServiceKey, L"ServiceDll", NULL, &ValueType, (LPBYTE)Buffer, &BufferSize); + if (RetVal != ERROR_SUCCESS || BufferSize == 0) + { + DPRINT1("Could not read 'ServiceDll' value from service: %ls, ErrorCode: 0x%lx\n", ServiceName, RetVal); + RegCloseKey(hServiceKey); + HeapFree(GetProcessHeap(), 0, Buffer); + return FALSE; + } + RegCloseKey(hServiceKey); + + if (ValueType == REG_EXPAND_SZ) + { + /* Convert possible %SystemRoot% to a real path */ + DllPathSize = ExpandEnvironmentStrings(Buffer, NULL, 0); + if (DllPathSize == 0) + { + DPRINT1("Invalid ServiceDll path: %ls, ErrorCode: %lu\n", Buffer, GetLastError()); HeapFree(GetProcessHeap(), 0, Buffer); - - Buffer = HeapAlloc(GetProcessHeap(), 0, BufferSize); - if (Buffer == NULL) - { - DPRINT1("Not enough memory for service: %s\n", ServiceName); return FALSE; } - - RetVal = RegQueryValueEx(hServiceKey, L"ServiceDll", NULL, &KeyType, (LPBYTE)Buffer, &BufferSize); - - } while (RetVal == ERROR_MORE_DATA); - - - RegCloseKey(hServiceKey); - - if (RetVal != ERROR_SUCCESS || BufferSize == 0) - { - DPRINT1("Could not read 'ServiceDll' value from service: %s, ErrorCode: 0x%x\n", ServiceName, RetVal); + DllPath = HeapAlloc(GetProcessHeap(), 0, DllPathSize * sizeof(WCHAR)); + if (DllPath == NULL) + { + DPRINT1("Not enough memory for service: %ls\n", ServiceName); + HeapFree(GetProcessHeap(), 0, Buffer); + return FALSE; + } + if (ExpandEnvironmentStrings(Buffer, DllPath, DllPathSize) != DllPathSize) + { + DPRINT1("Invalid ServiceDll path: %ls, ErrorCode: %lu\n", Buffer, GetLastError()); + HeapFree(GetProcessHeap(), 0, DllPath); + HeapFree(GetProcessHeap(), 0, Buffer); + return FALSE; + } + HeapFree(GetProcessHeap(), 0, Buffer); - return FALSE; - } - - /* Convert possible %SystemRoot% to a real path */ - BufferSize = ExpandEnvironmentStrings(Buffer, DllPath, _countof(DllPath)); - if (BufferSize == 0) - { - DPRINT1("Invalid ServiceDll path: %s\n", Buffer); + } + else if (ValueType == REG_SZ) + { + DllPath = Buffer; + } + else + { + DPRINT1("Invalid ServiceDll value type %lu for service: %ls, ErrorCode: %lu\n", ValueType, ServiceName, GetLastError()); HeapFree(GetProcessHeap(), 0, Buffer); return FALSE; } - - HeapFree(GetProcessHeap(), 0, Buffer);
/* Load the service dll */ DPRINT("Trying to load dll\n"); hServiceDll = LoadLibrary(DllPath); - if (hServiceDll == NULL) { - DPRINT1("Unable to load ServiceDll: %s, ErrorCode: %u\n", DllPath, GetLastError()); - return FALSE; - } + DPRINT1("Unable to load ServiceDll: %ls, ErrorCode: %lu\n", DllPath, GetLastError()); + HeapFree(GetProcessHeap(), 0, DllPath); + return FALSE; + } + HeapFree(GetProcessHeap(), 0, DllPath);
ServiceMainFunc = (LPSERVICE_MAIN_FUNCTION)GetProcAddress(hServiceDll, "ServiceMain");
/* Allocate a service node in the linked list */ - Service = HeapAlloc(GetProcessHeap(), 0, sizeof(SERVICE)); + Service = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(SERVICE)); if (Service == NULL) { - DPRINT1("Not enough memory for service: %s\n", ServiceName); - return FALSE; - } - - memset(Service, 0, sizeof(SERVICE)); - Service->Name = HeapAlloc(GetProcessHeap(), 0, (wcslen(ServiceName)+1) * sizeof(WCHAR)); + DPRINT1("Not enough memory for service: %ls\n", ServiceName); + return FALSE; + } + + ServiceNameSize = (wcslen(ServiceName) + 1) * sizeof(WCHAR); + Service->Name = HeapAlloc(GetProcessHeap(), 0, ServiceNameSize); if (Service->Name == NULL) { - DPRINT1("Not enough memory for service: %s\n", ServiceName); + DPRINT1("Not enough memory for service: %ls\n", ServiceName); HeapFree(GetProcessHeap(), 0, Service); return FALSE; } - wcscpy(Service->Name, ServiceName); + Result = StringCbCopy(Service->Name, ServiceNameSize, ServiceName); + ASSERT(SUCCEEDED(Result)); + (VOID)Result;
Service->hServiceDll = hServiceDll; Service->ServiceMainFunc = ServiceMainFunc; @@ -157,23 +195,26 @@ DWORD LoadServiceCategory(PCWSTR ServiceCategory) { HKEY hServicesKey; - DWORD KeyType; - DWORD BufferSize = REG_MAX_DATA_SIZE; + DWORD ValueType; WCHAR Buffer[REG_MAX_DATA_SIZE]; + DWORD BufferSize = sizeof(Buffer); PCWSTR ServiceName; DWORD BufferIndex = 0; DWORD NrOfServices = 0; + LONG RetVal;
/* Get all the services in this category */ - if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, SVCHOST_REG_KEY, 0, KEY_READ, &hServicesKey) != ERROR_SUCCESS) - { - DPRINT1("Could not open service category: %s\n", ServiceCategory); - return 0; - } - - if (RegQueryValueEx(hServicesKey, ServiceCategory, NULL, &KeyType, (LPBYTE)Buffer, &BufferSize) != ERROR_SUCCESS) - { - DPRINT1("Could not open service category (2): %s\n", ServiceCategory); + RetVal = RegOpenKeyEx(HKEY_LOCAL_MACHINE, SVCHOST_REG_KEY, 0, KEY_READ, &hServicesKey); + if (RetVal != ERROR_SUCCESS) + { + DPRINT1("Could not open service category: %ls\n", ServiceCategory); + return 0; + } + + RetVal = RegQueryValueEx(hServicesKey, ServiceCategory, NULL, &ValueType, (LPBYTE)Buffer, &BufferSize); + if (RetVal != ERROR_SUCCESS) + { + DPRINT1("Could not open service category (2): %ls\n", ServiceCategory); RegCloseKey(hServicesKey); return 0; } @@ -191,7 +232,7 @@ if (Length == 0) break;
- if (PrepareService(ServiceName) == TRUE) + if (PrepareService(ServiceName)) ++NrOfServices;
BufferIndex += Length + 1; @@ -213,7 +254,7 @@ return 0; }
- if (wcscmp(argv[1], L"-k") != 0) + if (wcscmp(argv[1], L"-k")) { /* For now, we only handle "-k" */ return 0; @@ -226,7 +267,6 @@ return 0;
ServiceTable = HeapAlloc(GetProcessHeap(), 0, sizeof(SERVICE_TABLE_ENTRY) * (NrOfServices + 1)); - if (ServiceTable != NULL) { DWORD i; @@ -235,17 +275,18 @@ /* Fill the service table */ for (i = 0; i < NrOfServices; i++) { - DPRINT("Loading service: %s\n", Service->Name); + DPRINT("Loading service: %ls\n", Service->Name); ServiceTable[i].lpServiceName = Service->Name; ServiceTable[i].lpServiceProc = Service->ServiceMainFunc; Service = Service->Next; } + ASSERT(Service == NULL);
/* Set a NULL entry to end the service table */ ServiceTable[i].lpServiceName = NULL; ServiceTable[i].lpServiceProc = NULL;
- if (StartServiceCtrlDispatcher(ServiceTable) == FALSE) + if (!StartServiceCtrlDispatcher(ServiceTable)) DPRINT1("Failed to start service control dispatcher, ErrorCode: %lu\n", GetLastError());
HeapFree(GetProcessHeap(), 0, ServiceTable);
Modified: trunk/reactos/base/services/svchost/svchost.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/svchost/svcho... ============================================================================== --- trunk/reactos/base/services/svchost/svchost.h [iso-8859-1] (original) +++ trunk/reactos/base/services/svchost/svchost.h [iso-8859-1] Sat Jun 29 13:01:35 2013 @@ -10,20 +10,15 @@
/* INCLUDES ******************************************************************/
-#if 0 -#define _CRT_SECURE_NO_DEPRECATE 1 -#endif - #include <stdlib.h> #include <stdarg.h> #include <windef.h> #include <winbase.h> #include <winreg.h> #include <winsvc.h> +#include <strsafe.h>
/* DEFINES *******************************************************************/ - -#define CS_TIMEOUT 1000
typedef struct _SERVICE { PWSTR Name;
-
tfaber＠svn.reactos.org