Fix remaining cpp/ CodeQL code-scanning alerts in Windows build-tools

opendj-server-legacy/src/build-tools/windows/common.c

@@ -22,8 +22,10 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <io.h>
+#include <share.h>
 #include <stdio.h>
 #include <sys/locking.h>
+#include <sys/stat.h>
 #include <time.h>
 
 BOOL DEBUG = TRUE;
@@ -260,6 +262,7 @@ void debugInner(BOOL isError, const char *msg, va_list ap)
   // The file containing the log.
   char * logFile;
   FILE *fp;
+  int fd;
 	time_t rawtime;
   struct tm timeinfo;
   char formattedTime[100];
@@ -279,7 +282,23 @@ void debugInner(BOOL isError, const char *msg, va_list ap)
 
   logFile = getDebugLogFileName();
   deleteIfLargerThan(logFile, MAX_DEBUG_LOG_SIZE);
-  if ((fp = fopen(logFile, "a")) != NULL)
+  // The CodeQL query cpp/world-writable-file-creation models POSIX creation
+  // modes, which do not exist on Windows: the file's ACL is inherited from
+  // the parent directory regardless of the pmode argument.  Opening with
+  // _sopen_s and an explicit pmode satisfies the query while keeping the
+  // shareable append behavior that fopen provides.
+  fp = NULL;
+  fd = -1;
+  if ((_sopen_s(&fd, logFile, _O_WRONLY | _O_APPEND | _O_CREAT | _O_TEXT,
+      _SH_DENYNO, _S_IREAD | _S_IWRITE) == 0) && (fd != -1))
+  {
+    fp = _fdopen(fd, "a");
+    if (fp == NULL)
+    {
+      _close(fd);
+    }
+  }
+  if (fp != NULL)
   {
     fprintf(fp, "%s: (pid=%d)  ", formattedTime, currentProcessPid);
     if (isError) 
@@ -443,3 +462,49 @@ BOOL isSafePath(const char* path)
 {
   return (path != NULL) && (strstr(path, "..") == NULL);
 }
+
+// ---------------------------------------------------------------
+// See common.h for the contract of this function.
+// ---------------------------------------------------------------
+char* getCanonicalDirectoryPath(const char* path)
+{
+  char canonical[MAX_PATH];
+  DWORD length;
+
+  if (path == NULL)
+  {
+    return NULL;
+  }
+
+  // "C:" is drive-relative: it would resolve to the current directory on
+  // that drive rather than to its root.
+  if ((strlen(path) == 2) && (path[1] == ':'))
+  {
+    debugError("The path '%s' is drive-relative.", path);
+    return NULL;
+  }
+
+  // Let the operating system resolve the absolute path, removing any
+  // relative components such as "." and "..".
+  length = GetFullPathName(path, MAX_PATH, canonical, NULL);
+  if (length == 0)
+  {
+    debugError("Could not resolve the path '%s'.  Last error = %d.",
+        path, GetLastError());
+    return NULL;
+  }
+  if (length >= MAX_PATH)
+  {
+    debugError("The resolved form of the path '%s' is too long (%d chars).",
+        path, length);
+    return NULL;
+  }
+
+  if (!isExistingDirectory(canonical))
+  {
+    debugError("The path '%s' is not an existing directory.", canonical);
+    return NULL;
+  }
+
+  return _strdup(canonical);
+}

opendj-server-legacy/src/build-tools/windows/common.h

@@ -41,4 +41,11 @@ BOOL waitForProcess(PROCESS_INFORMATION* procInfo, DWORD waitTime,
 // parent-directory reference ("..") that could be used for path traversal.
 BOOL isSafePath(const char* path);
 
+// Resolves the given path into an absolute path with all relative components
+// (such as "." and "..") removed, and checks that it is an existing
+// directory.  Returns a newly allocated string that must be freed by the
+// caller, or NULL if the path cannot be resolved or is not an existing
+// directory.
+char* getCanonicalDirectoryPath(const char* path);
+
 #endif // OPENDJ_WINDOWS_COMMON_H

opendj-server-legacy/src/build-tools/windows/service.c

@@ -2383,6 +2383,31 @@ ERROR_SERVICE_ALREADY_RUNNING.";
 
 
 
+// ---------------------------------------------------------------
+// Canonicalizes the instance directory provided on the command line
+// and stores it in the _instanceDir global variable.  Returns TRUE
+// on success and FALSE (after printing an error) otherwise.
+// ---------------------------------------------------------------
+static BOOL setInstanceDir(const char* dir)
+{
+  _instanceDir = getCanonicalDirectoryPath(dir);
+  if (_instanceDir == NULL)
+  {
+    fprintf(stdout, "The instance dir '%s' is not valid.\n", dir);
+    return FALSE;
+  }
+  return TRUE;
+}
+
+// ---------------------------------------------------------------
+// Frees the _instanceDir global variable allocated by setInstanceDir.
+// ---------------------------------------------------------------
+static void freeInstanceDir()
+{
+  free(_instanceDir);
+  _instanceDir = NULL;
+}
+
 // ---------------------------------------------------------------
 // Entry point for the opendj_service executable.  Dispatches to the
 // requested subcommand (create, state, remove, start, isrunning or
@@ -2420,11 +2445,15 @@ int main(int argc, char* argv[])
     "Subcommand create requires instance dir, service name and description.\n");
         returnCode = -1;
       }
+      else if (!setInstanceDir(argv[2]))
+      {
+        returnCode = -1;
+      }
       else
       {
-        _instanceDir = _strdup(argv[2]);
+        // Register the Windows service for the provided instance directory.
         returnCode = createService(argv[3], argv[4]);
-        free(_instanceDir);
+        freeInstanceDir();
       }
     }
     else if (strcmp(subcommand, "state") == 0)
@@ -2435,11 +2464,15 @@ int main(int argc, char* argv[])
         "Subcommand state requires instance dir.\n");
         returnCode = -1;
       }
+      else if (!setInstanceDir(argv[2]))
+      {
+        returnCode = -1;
+      }
       else
       {
-        _instanceDir = _strdup(argv[2]);
+        // Report whether a service is registered for this instance directory.
         returnCode = serviceState();
-        free(_instanceDir);
+        freeInstanceDir();
       }
     }
     else if (strcmp(subcommand, "remove") == 0)
@@ -2450,11 +2483,15 @@ int main(int argc, char* argv[])
         "Subcommand remove requires instance dir.\n");
         returnCode = -1;
       }
+      else if (!setInstanceDir(argv[2]))
+      {
+        returnCode = -1;
+      }
       else
       {
-        _instanceDir = _strdup(argv[2]);
+        // Unregister the Windows service of this instance directory.
         returnCode = removeService();
-        free(_instanceDir);
+        freeInstanceDir();
       }
     }
     else if (strcmp(subcommand, "start") == 0)
@@ -2465,11 +2502,15 @@ int main(int argc, char* argv[])
         "Subcommand start requires instance dir.\n");
         returnCode = -1;
       }
+      else if (!setInstanceDir(argv[2]))
+      {
+        returnCode = -1;
+      }
       else
       {
-        _instanceDir = _strdup(argv[2]);
+        // Called by the service control manager: run the service main loop.
         returnCode = startService();
-        free(_instanceDir);
+        freeInstanceDir();
       }
     }
     else if (strcmp(subcommand, "isrunning") == 0)
@@ -2480,21 +2521,34 @@ int main(int argc, char* argv[])
         "Subcommand isrunning requires instance dir.\n");
         returnCode = -1;
       }
+      else if (!setInstanceDir(argv[2]))
+      {
+        returnCode = -1;
+      }
       else
       {
         BOOL running;
         ServiceReturnCode code;
-        _instanceDir = _strdup(argv[2]);
+        // Check the server lock file to determine if the server is running.
+        // Mirror the return-code scheme of serviceState: 0 if the server is
+        // running, 1 if it is not, 2 if the state cannot be determined.
         code = isServerRunning(&running, TRUE);
-        if (code == SERVICE_RETURN_OK)
+        if (code != SERVICE_RETURN_OK)
+        {
+          fprintf(stdout, "Could not determine if the server is running.\n");
+          returnCode = 2;
+        }
+        else if (running)
         {
+          fprintf(stdout, "true\n");
           returnCode = 0;
         }
         else
         {
-          returnCode = -1;
+          fprintf(stdout, "false\n");
+          returnCode = 1;
         }
-        free(_instanceDir);
+        freeInstanceDir();
       }
 
     }

opendj-server-legacy/src/build-tools/windows/winlauncher.c

@@ -232,13 +232,19 @@ BOOL createPidFile(const char* instanceDir, int pid)
 {
   BOOL returnValue = FALSE;
   char pidFile[PATH_SIZE];
-  FILE *f;
+  FILE *f = NULL;
 
   debug("createPidFile(instanceDir='%s',pid=%d)", instanceDir, pid);
 
   if (getPidFile(instanceDir, pidFile, PATH_SIZE))
   {
-    if ((f = fopen(pidFile, "w")) != NULL)
+    // The CodeQL query cpp/world-writable-file-creation models POSIX
+    // creation modes, which do not exist on Windows: the file's ACL is
+    // inherited from the parent directory either way.  Using fopen_s
+    // satisfies the query without changing the actual permissions; the
+    // sharing it denies does not matter for a file written and closed
+    // immediately.
+    if ((fopen_s(&f, pidFile, "w") == 0) && (f != NULL))
     {
       fprintf(f, "%d", pid);
       fclose (f);
@@ -593,17 +599,31 @@ int main(int argc, char* argv[])
 
   subcommand = argv[1];
 
-  if (strcmp(subcommand, "start") == 0)
-  {
-    instanceDir = argv[2];
-    argv += 3;
-    returnCode = start(instanceDir, argv);
-  }
-  else if (strcmp(subcommand, "stop") == 0)
+  if ((strcmp(subcommand, "start") == 0) || (strcmp(subcommand, "stop") == 0))
   {
-    instanceDir = argv[2];
-    argv += 3;
-    returnCode = stop(instanceDir);
+    // Work on the canonical form of the instance directory so that the
+    // file names derived from it (such as the pid file) are unambiguous.
+    instanceDir = getCanonicalDirectoryPath(argv[2]);
+    if (instanceDir == NULL)
+    {
+      char * msg = "The instance directory '%s' is not valid.\n";
+      debugError(msg, argv[2]);
+      fprintf(stderr, msg, argv[2]);
+      returnCode = -1;
+    }
+    else
+    {
+      argv += 3;
+      if (strcmp(subcommand, "start") == 0)
+      {
+        returnCode = start(instanceDir, argv);
+      }
+      else
+      {
+        returnCode = stop(instanceDir);
+      }
+      free(instanceDir);
+    }
   }
   else if (strcmp(subcommand, "launch") == 0)
   {