Browse Source

Fix crash on Windows XP.

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.
Iain Patterson 5 years ago
parent
commit
095331018c
4 changed files with 13 additions and 2 deletions
  1. 1 0
      README.txt
  2. 9 0
      env.cpp
  3. 1 0
      env.h
  4. 2 2
      service.cpp

+ 1 - 0
README.txt

@@ -878,6 +878,7 @@ 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.
+Thanks to Scott Ware for reporting a crash saving the environment on XP 32-bit.
 
 Licence
 -------

+ 9 - 0
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;
+}

+ 1 - 0
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

+ 2 - 2
service.cpp

@@ -774,7 +774,7 @@ void cleanup_nssm_service(nssm_service_t *service) {
   if (service->throttle_section_initialised) DeleteCriticalSection(&service->throttle_section);
   if (service->throttle_timer) CloseHandle(service->throttle_timer);
   if (service->hook_section_initialised) DeleteCriticalSection(&service->hook_section);
-  if (service->initial_env) FreeEnvironmentStrings(service->initial_env);
+  if (service->initial_env) HeapFree(GetProcessHeap(), 0, service->initial_env);
   HeapFree(GetProcessHeap(), 0, service);
 }
 
@@ -1479,7 +1479,7 @@ void WINAPI service_main(unsigned long argc, TCHAR **argv) {
   service->hook_section_initialised = true;
 
   /* Remember our initial environment. */
-  service->initial_env = GetEnvironmentStrings();
+  service->initial_env = copy_environment();
 
   /* Remember our creation time. */
   if (get_process_creation_time(GetCurrentProcess(), &service->nssm_creation_time)) ZeroMemory(&service->nssm_creation_time, sizeof(service->nssm_creation_time));