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.
izenkov · Mar 2, 2015 · fb96938 · fb96938
1 parent 4c25687
commit fb96938
Show file tree

Hide file tree

Showing 6 changed files with 8 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);