Fix crash on Windows XP.
authorIain Patterson <me@iain.cx>
Wed, 17 Feb 2016 09:42:16 +0000 (09:42 +0000)
committerIain Patterson <me@iain.cx>
Wed, 17 Feb 2016 09:42:16 +0000 (09:42 +0000)
The pointer returned by GetEnvironmentStrings() on Unicode versions of
Windows XP was to the raw environment block, not a copy as described in
the documentation.  Retrieving it into service->initial_env and keeping
it around for the lifetime of the service could cause a crash.  Instead
we now copy the memory and immediately call FreeEnvironmentStrings().

Thanks Scott Ware.

README.txt
env.cpp
env.h
service.cpp

index 12db7e8..d78d854 100644 (file)
@@ -878,6 +878,7 @@ Thanks to Yuriy Lesiuk for suggesting setting the environment before querying
 the registry for parameters.\r
 Thanks to Gerald Haider for noticing that installing a service with NSSM in a\r
 path containing spaces was technically a security vulnerability.\r
+Thanks to Scott Ware for reporting a crash saving the environment on XP 32-bit.\r
 \r
 Licence\r
 -------\r
diff --git a/env.cpp b/env.cpp
index 083f46a..a1ca900 100644 (file)
--- a/env.cpp
+++ b/env.cpp
@@ -170,3 +170,12 @@ void duplicate_environment_strings(TCHAR *env) {
   duplicate_environment(newenv);
   HeapFree(GetProcessHeap(), 0, newenv);
 }
+
+/* Safely get a copy of the current environment. */
+TCHAR *copy_environment() {
+  TCHAR *rawenv = GetEnvironmentStrings();
+  if (! rawenv) return NULL;
+  TCHAR *env = copy_environment_block(rawenv);
+  FreeEnvironmentStrings(rawenv);
+  return env;
+}
diff --git a/env.h b/env.h
index 021ff4f..b690106 100644 (file)
--- a/env.h
+++ b/env.h
@@ -9,5 +9,6 @@ int clear_environment();
 int duplicate_environment(TCHAR *);
 int test_environment(TCHAR *);
 void duplicate_environment_strings(TCHAR *);
+TCHAR *copy_environment();
 
 #endif
index 6a3907e..b24eae2 100644 (file)
@@ -774,7 +774,7 @@ void cleanup_nssm_service(nssm_service_t *service) {
   if (service->throttle_section_initialised) DeleteCriticalSection(&service->throttle_section);\r
   if (service->throttle_timer) CloseHandle(service->throttle_timer);\r
   if (service->hook_section_initialised) DeleteCriticalSection(&service->hook_section);\r
-  if (service->initial_env) FreeEnvironmentStrings(service->initial_env);\r
+  if (service->initial_env) HeapFree(GetProcessHeap(), 0, service->initial_env);\r
   HeapFree(GetProcessHeap(), 0, service);\r
 }\r
 \r
@@ -1479,7 +1479,7 @@ void WINAPI service_main(unsigned long argc, TCHAR **argv) {
   service->hook_section_initialised = true;\r
 \r
   /* Remember our initial environment. */\r
-  service->initial_env = GetEnvironmentStrings();\r
+  service->initial_env = copy_environment();\r
 \r
   /* Remember our creation time. */\r
   if (get_process_creation_time(GetCurrentProcess(), &service->nssm_creation_time)) ZeroMemory(&service->nssm_creation_time, sizeof(service->nssm_creation_time));\r