Ensure the directory used for batch job step scripts is not predictable.

CVE-2025-0218

Per report from Wolfgang Frisch.

Author: asheshv

include/misc.h

@@ -23,7 +23,7 @@ std::wstring  s2ws(const std::string &str);
 std::string   ws2s(const std::wstring &wstr);
 #endif
 std::string   generateRandomString(size_t length);
-std::string   getTemporaryDirectoryPath();
+bool          createUniqueTemporaryDirectory(const std::string &prefix, boost::filesystem::path &tempDir);
 
 class MutexLocker
 {

job.cpp

@@ -163,45 +163,41 @@ int Job::Execute()
           LOG_DEBUG
 				);
 
-				// Get a temporary filename, then reuse it to create an empty directory.
-				std::string sDirectory = getTemporaryDirectoryPath();
-				std::string sFilesName = std::string("");
-				std::string prefix = std::string("pga_");
-
-				// Generate random string of 6 characters long to make unique dir name
-				std::string result = generateRandomString(7);
-				sFilesName = prefix + m_jobid + std::string("_") + stepid + std::string("_") + result;
+				namespace fs = boost::filesystem;
+
+				// Generate unique temporary directory
+				std::string prefix = (
+					boost::format("pga_%s_%s_") % m_jobid % stepid
+				).str();
+
+				fs::path jobDir;
+				fs::path filepath((
+					boost::format("%s_%s.%s") %
+					m_jobid % stepid %
 #if BOOST_OS_WINDOWS
-				std::string sModel = (boost::format("%s\\%s") % sDirectory % sFilesName).str();
+					".bat"
 #else
-				std::string sModel = (boost::format("%s/%s") % sDirectory % sFilesName).str();
+					".scr"
 #endif
-				std::string dirname = sModel;
+				).str());
+				fs::path errorFilePath(
+					(boost::format("%s_%s_error.txt") % m_jobid % stepid).str()
+				);
 
-				if (dirname == "")
+				if (!createUniqueTemporaryDirectory(prefix, jobDir))
 				{
 					output = "Couldn't get a temporary filename!";
 					LogMessage(output, LOG_WARNING);
 					rc = -1;
-					break;
-				}
 
-				if (!boost::filesystem::create_directory(boost::filesystem::path(dirname)))
-				{
-					LogMessage(
-						"Couldn't create temporary directory: " + dirname, LOG_WARNING
-					);
-					rc = -1;
 					break;
 				}
 
-#if BOOST_OS_WINDOWS
-				std::string filename = dirname + "\\" + m_jobid + "_" + stepid + ".bat";
-				std::string errorFile = dirname + "\\" + m_jobid + "_" + stepid + "_error.txt";
-#else
-				std::string filename = dirname + "/" + m_jobid + "_" + stepid + ".scr";
-				std::string errorFile = dirname + "/" + m_jobid + "_" + stepid + "_error.txt";
-#endif
+				filepath = jobDir / filepath;
+				errorFilePath = jobDir / errorFilePath;
+
+				std::string filename = filepath.string();
+				std::string errorFile = errorFilePath.string();
 
 				std::string code = steps->GetString("jstcode");
 
@@ -222,8 +218,8 @@ int Job::Execute()
 						LOG_WARNING
 					);
 
-					if (boost::filesystem::exists(dirname))
-						boost::filesystem::remove_all(dirname);
+					if (boost::filesystem::exists(jobDir))
+						boost::filesystem::remove_all(jobDir);
 
 					rc = -1;
 					break;
@@ -234,14 +230,17 @@ int Job::Execute()
 					out_file.close();
 
 #if !BOOST_OS_WINDOWS
-					// change file permission to 700 for executable in linux
-					int ret = chmod((const char *)filename.c_str(), S_IRWXU);
-
-					if (ret != 0)
+					// Change file permission to 700 for executable in linux
+					try {
+						boost::filesystem::permissions(
+							filepath, boost::filesystem::owner_all
+						);
+					} catch (const fs::filesystem_error &ex) {
 						LogMessage(
-							"Error setting executable permission to file: " + filename,
-							LOG_DEBUG
+							"Error setting executable permission to file: " +
+							filename, LOG_DEBUG
 						);
+					}
 #endif
 				}
 
@@ -368,10 +367,8 @@ int Job::Execute()
 				// output in the log, just throw warnings.
 				try
 				{
-					boost::filesystem::path dir_path(dirname);
-
-					if (boost::filesystem::exists(dir_path))
-						boost::filesystem::remove_all(dir_path);
+					if (boost::filesystem::exists(jobDir))
+						boost::filesystem::remove_all(jobDir);
 				}
 				catch (boost::filesystem::filesystem_error const & e)
 				{

misc.cpp

@@ -21,6 +21,8 @@
 
 #define APPVERSION_STR PGAGENT_VERSION
 
+namespace fs = boost::filesystem;
+
 // In unix.c or win32.c
 void usage(const std::string &executable);
 
@@ -49,7 +51,7 @@ void printVersion()
 {
 	printf("PostgreSQL Scheduling Agent\n");
 	printf("Version: %s\n", APPVERSION_STR);
-}
+};
 
 void setOptions(int argc, char **argv, const std::string &executable)
 {
@@ -192,41 +194,37 @@ std::string generateRandomString(size_t length)
 	return result;
 }
 
-std::string getTemporaryDirectoryPath()
+bool createUniqueTemporaryDirectory(const std::string &prefix, fs::path &uniqueDir)
 {
+	const unsigned short MAX_ATTEMPTS = 100;
+	unsigned short attempts = 0;
 
-#if BOOST_OS_WINDOWS
-    std::wstring tmp_dir;
+	try {
+		fs::path tempDir = fs::temp_directory_path();
 
-    wchar_t wcharPath[MAX_PATH];
+		do {
+			if (attempts++ >= MAX_ATTEMPTS)
+				return false;
 
-    if (GetTempPathW(MAX_PATH, wcharPath))
-		{
-        tmp_dir = wcharPath;
+			uniqueDir = tempDir / fs::unique_path(
+				prefix + "%%%%%%%%-%%%%-%%%%-%%%%-%%%%%%%%%%%%"
+			);
 
-				return ws2s(tmp_dir);
-		}
-    return "";
-#else
-    // Read this environment variable (TMPDIR, TMP, TEMP, TEMPDIR) and if not found then use "/tmp"
-    std::string tmp_dir = "/tmp";
-    const char *s_tmp = getenv("TMPDIR");
+			// Check if exists
+			if (boost::filesystem::is_directory(uniqueDir))
+				continue;
 
-    if (s_tmp != NULL)
-        return s_tmp;
-
-		s_tmp = getenv("TMP");
-		if (s_tmp != NULL)
-			return s_tmp;
-
-		s_tmp = getenv("TEMP");
-		if (s_tmp != NULL)
-			return s_tmp;
+			// Create the directory securely
+			if (!fs::create_directory(uniqueDir)) {
+				return false;
+			}
 
-		s_tmp = getenv("TEMPDIR");
-		if (s_tmp != NULL)
-			return s_tmp;
+			// Set appropriate permissions (example: owner read/write/execute only)
+			fs::permissions(uniqueDir, boost::filesystem::owner_all);
 
-		return tmp_dir;
-#endif
+			return true;
+		} while (true);
+	} catch (const fs::filesystem_error &ex) {
+		return false;
+	}
 }