From a6d5d2dcc36cf40aca64df6bf16fbfb8a91e6893 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Mon, 27 Jul 2026 13:03:09 +0000
Subject: [PATCH] Fix remaining cpp/ CodeQL code-scanning alerts in Windows build-tools (#762)
---
opendj-server-legacy/src/build-tools/windows/winlauncher.c | 44 ++++++++---
opendj-server-legacy/src/build-tools/windows/common.h | 7 +
opendj-server-legacy/src/build-tools/windows/service.c | 78 ++++++++++++++++---
opendj-server-legacy/src/build-tools/windows/common.c | 67 ++++++++++++++++
4 files changed, 171 insertions(+), 25 deletions(-)
diff --git a/opendj-server-legacy/src/build-tools/windows/common.c b/opendj-server-legacy/src/build-tools/windows/common.c
index bd62f08..7a97dc1 100644
--- a/opendj-server-legacy/src/build-tools/windows/common.c
+++ b/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);
+}
diff --git a/opendj-server-legacy/src/build-tools/windows/common.h b/opendj-server-legacy/src/build-tools/windows/common.h
index 405e405..2fb2a80 100644
--- a/opendj-server-legacy/src/build-tools/windows/common.h
+++ b/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
diff --git a/opendj-server-legacy/src/build-tools/windows/service.c b/opendj-server-legacy/src/build-tools/windows/service.c
index 587cc32..23a7649 100644
--- a/opendj-server-legacy/src/build-tools/windows/service.c
+++ b/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();
}
}
diff --git a/opendj-server-legacy/src/build-tools/windows/winlauncher.c b/opendj-server-legacy/src/build-tools/windows/winlauncher.c
index e449a13..ca301b6 100644
--- a/opendj-server-legacy/src/build-tools/windows/winlauncher.c
+++ b/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)
{
--
Gitblit v1.10.0