diff --git a/opendj-server-legacy/src/build-tools/windows/common.c b/opendj-server-legacy/src/build-tools/windows/common.c index bd62f08ad3..7a97dc1414 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 #include #include +#include #include #include +#include #include 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); +} diff --git a/opendj-server-legacy/src/build-tools/windows/common.h b/opendj-server-legacy/src/build-tools/windows/common.h index 405e40514f..2fb2a808b7 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 @@ 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 diff --git a/opendj-server-legacy/src/build-tools/windows/service.c b/opendj-server-legacy/src/build-tools/windows/service.c index 587cc322bf..23a7649dd9 100644 --- a/opendj-server-legacy/src/build-tools/windows/service.c +++ b/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(); } } diff --git a/opendj-server-legacy/src/build-tools/windows/winlauncher.c b/opendj-server-legacy/src/build-tools/windows/winlauncher.c index e449a137e1..ca301b6068 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 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) {