Fix remaining cpp/ CodeQL code-scanning alerts in Windows build-tools (#762)

Valery Kharseko

23 hours ago a6d5d2dcc36cf40aca64df6bf16fbfb8a91e6893

Fix remaining cpp/ CodeQL code-scanning alerts in Windows build-tools (#762)

 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 @@
  // The file containing the log.
  char * logFile;
  FILE *fp;
  int fd;
    time_t rawtime;
  struct tm timeinfo;
  char formattedTime[100];
@@ -279,7 +282,23 @@
  
  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 @@
{
  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 @@
// 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

@@ -2384,6 +2384,31 @@


// ---------------------------------------------------------------
// 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
// cleanup); each operates on the OpenDJ instance directory (or the
@@ -2420,11 +2445,15 @@
    "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 @@
        "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 @@
        "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 @@
        "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 @@
        "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 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 @@

  subcommand = argv[1];

  if (strcmp(subcommand, "start") == 0)
  if ((strcmp(subcommand, "start") == 0) || (strcmp(subcommand, "stop") == 0))
  {
    instanceDir = argv[2];
    argv += 3;
    returnCode = start(instanceDir, argv);
  }
  else if (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)
  {