From fb96938cf944edf3bc0dfd99dbff416b0397df4f Mon Sep 17 00:00:00 2001 From: Iain Patterson Date: Mon, 2 Mar 2015 11:03:59 +0000 Subject: [PATCH] Use nssm_imagepath(). We were calling GetModuleFileName() in a number of places. Use nssm_imagepath() instead. Ensure that a quoted path is used for the service image path when creating the service, thus avoiding a theoretical security vulnerability whereby the service manager could be tricked into launching the wrong program. When setting the path to NSSM in the environment for event hooks we still use the unquoted path, as environment variables are typically unquoted. Thanks Gerald Haider. --- README.txt | 2 ++ env.cpp | 3 +-- hook.cpp | 6 ++---- nssm.cpp | 9 +-------- registry.cpp | 3 +-- service.cpp | 2 +- 6 files changed, 8 insertions(+), 17 deletions(-) diff --git a/README.txt b/README.txt index ecca5e9..f48a7f1 100644 --- a/README.txt +++ b/README.txt @@ -822,6 +822,8 @@ application's child processes. Thanks to Miguel Angel Terrón for suggesting copy/truncate rotation. Thanks to Yuriy Lesiuk for suggesting setting the environment before querying the registry for parameters. +Thanks to Gerald Haider for noticing that installing a service with NSSM in a +path containing spaces was technically a security vulnerability. Licence ------- diff --git a/env.cpp b/env.cpp index 9945d20..083f46a 100644 --- a/env.cpp +++ b/env.cpp @@ -129,8 +129,7 @@ int duplicate_environment(TCHAR *rawenv) { -1 on error. */ int test_environment(TCHAR *env) { - TCHAR path[PATH_LENGTH]; - GetModuleFileName(0, path, _countof(path)); + TCHAR *path = (TCHAR *) nssm_imagepath(); STARTUPINFO si; ZeroMemory(&si, sizeof(si)); si.cb = sizeof(si); diff --git a/hook.cpp b/hook.cpp index 09b98d9..780590b 100644 --- a/hook.cpp +++ b/hook.cpp @@ -254,10 +254,8 @@ int nssm_hook(hook_thread_t *hook_threads, nssm_service_t *service, TCHAR *hook_ /* Last control handled. */ SetEnvironmentVariable(NSSM_HOOK_ENV_LAST_CONTROL, service_control_text(service->last_control)); - /* Path to NSSM. */ - TCHAR path[PATH_LENGTH]; - GetModuleFileName(0, path, _countof(path)); - SetEnvironmentVariable(NSSM_HOOK_ENV_IMAGE_PATH, path); + /* Path to NSSM, unquoted for the environment. */ + SetEnvironmentVariable(NSSM_HOOK_ENV_IMAGE_PATH, nssm_unquoted_imagepath()); /* NSSM version. */ SetEnvironmentVariable(NSSM_HOOK_ENV_NSSM_CONFIGURATION, NSSM_CONFIGURATION); diff --git a/nssm.cpp b/nssm.cpp index ddd1561..0b586f3 100644 --- a/nssm.cpp +++ b/nssm.cpp @@ -65,16 +65,10 @@ static int elevate(int argc, TCHAR **argv, unsigned long message) { ZeroMemory(&sei, sizeof(sei)); sei.cbSize = sizeof(sei); sei.lpVerb = _T("runas"); - sei.lpFile = (TCHAR *) HeapAlloc(GetProcessHeap(), 0, PATH_LENGTH); - if (! sei.lpFile) { - print_message(stderr, NSSM_MESSAGE_OUT_OF_MEMORY, _T("GetModuleFileName()"), _T("elevate()")); - return 111; - } - GetModuleFileName(0, (TCHAR *) sei.lpFile, PATH_LENGTH); + sei.lpFile = (TCHAR *) nssm_imagepath(); TCHAR *args = (TCHAR *) HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, EXE_LENGTH * sizeof(TCHAR)); if (! args) { - HeapFree(GetProcessHeap(), 0, (void *) sei.lpFile); print_message(stderr, NSSM_MESSAGE_OUT_OF_MEMORY, _T("GetCommandLine()"), _T("elevate()")); return 111; } @@ -91,7 +85,6 @@ static int elevate(int argc, TCHAR **argv, unsigned long message) { unsigned long exitcode = 0; if (! ShellExecuteEx(&sei)) exitcode = 100; - HeapFree(GetProcessHeap(), 0, (void *) sei.lpFile); HeapFree(GetProcessHeap(), 0, (void *) args); return exitcode; } diff --git a/registry.cpp b/registry.cpp index de3127f..5a4c652 100644 --- a/registry.cpp +++ b/registry.cpp @@ -17,8 +17,7 @@ int create_messages() { } /* Get path of this program */ - TCHAR path[PATH_LENGTH]; - GetModuleFileName(0, path, _countof(path)); + const TCHAR *path = nssm_imagepath(); /* Try to register the module but don't worry so much on failure */ RegSetValueEx(key, _T("EventMessageFile"), 0, REG_SZ, (const unsigned char *) path, (unsigned long) (_tcslen(path) + 1) * sizeof(TCHAR)); diff --git a/service.cpp b/service.cpp index 617cad1..6a3907e 100644 --- a/service.cpp +++ b/service.cpp @@ -1126,7 +1126,7 @@ int install_service(nssm_service_t *service) { } /* Get path of this program */ - GetModuleFileName(0, service->image, _countof(service->image)); + _sntprintf_s(service->image, _countof(service->image), _TRUNCATE, _T("%s"), nssm_imagepath()); /* Create the service - settings will be changed in edit_service() */ service->handle = CreateService(services, service->name, service->name, SERVICE_ALL_ACCESS, SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START, SERVICE_ERROR_NORMAL, service->image, 0, 0, 0, 0, 0); -- 2.7.4