File U_13-acct_gather_profile-hdf5-Avoid-TOCTOU-abus... of Package slurm

Overview Repositories Revisions Requests Users Attributes Meta

File U_13-acct_gather_profile-hdf5-Avoid-TOCTOU-abuse-when-creating-directories.patch of Package slurm

From: Alejandro Sanchez <alex@schedmd.com>
Date: Wed Oct 11 12:45:25 2023 -0600
Subject: [PATCH 13/19]acct_gather_profile/hdf5 - Avoid TOCTOU abuse when creating directories.
Patch-mainline: Upstream
Git-repo: https://github.com/SchedMD/slurm
Git-commit: ca6cba82d35efc2a74345ae88b8e5b3ccbace47b
References: CVE-2022-29500, bsc#1216207
Signed-off-by: Egbert Eich <eich@suse.de>

Prevent TOCTOU abuse by using the *at family of syscalls (which work on file
descriptors instead of path names) when creating and changing permissions
for ProfileHDF5Dir and its user-specific sub-directories.
---
 .../hdf5/acct_gather_profile_hdf5.c                | 82 ++++++++++++++--------
 1 file changed, 51 insertions(+), 31 deletions(-)

diff --git a/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c b/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c
index e1226ad5fb..bff2b38173 100644
--- a/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c
+++ b/src/plugins/acct_gather_profile/hdf5/acct_gather_profile_hdf5.c
@@ -156,46 +156,66 @@ static uint32_t _determine_profile(void)
 
 static int _create_directories(void)
 {
-	int rc;
-	struct stat st;
-	char   *user_dir = NULL;
+	char *parent_dir = NULL, *user_dir = NULL, *hdf5_dir_rel = NULL;
+	char *slash = NULL;
+	int parent_dirfd, user_parent_dirfd;
 
 	xassert(g_job);
 	xassert(hdf5_conf.dir);
+
+	parent_dir = xstrdup(hdf5_conf.dir);
+	/* split into base and new directory name */
+	while ((slash = strrchr(parent_dir, '/'))) {
+		/* fix a path with one or more trailing slashes */
+		if (slash[1] == '\0')
+			slash[0] = '\0';
+		else
+			break;
+	}
+
+	if (!slash)
+		fatal("Invalid ProfileHDF5Dir=\"%s\"", hdf5_conf.dir);
+
+	slash[0] = '\0';
+	hdf5_dir_rel = slash + 1;
+
+	parent_dirfd = open(parent_dir, O_DIRECTORY | O_NOFOLLOW);
+
 	/*
-	 * If profile director does not exist, try to create it.
-	 *  Otherwise, ensure path is a directory as expected, and that
-	 *  we have permission to write to it.
-	 *  also make sure the subdirectory tmp exists.
+	 * Use *at family of syscalls to prevent TOCTOU abuse by working
+	 * on file descriptors instead of path names.
 	 */
+	if ((mkdirat(parent_dirfd, hdf5_dir_rel, 0755)) < 0) {
+		/* Never chmod on EEXIST */
+		if (errno != EEXIST)
+			fatal("mkdirat(%s): %m", hdf5_conf.dir);
+	} else if (fchmodat(parent_dirfd, hdf5_dir_rel, 0755,
+			    AT_SYMLINK_NOFOLLOW) < 0)
+		fatal("fchmodat(%s): %m", hdf5_conf.dir);
+
+	xstrfmtcat(user_dir, "%s/%s", hdf5_conf.dir, g_job->user_name);
+	user_parent_dirfd = openat(parent_dirfd, hdf5_dir_rel,
+				   O_DIRECTORY | O_NOFOLLOW);
+	close(parent_dirfd);
+
+	if ((mkdirat(user_parent_dirfd, g_job->user_name, 0700)) < 0) {
+		/* Never chmod on EEXIST */
+		if (errno != EEXIST)
+			fatal("mkdirat(%s): %m", user_dir);
+	} else {
+		/* fchmodat(2) man says AT_SYMLINK_NOFOLLOW not implemented. */
+		if (fchmodat(user_parent_dirfd, g_job->user_name, 0700, 0) < 0)
+			fatal("fchmodat(%s): %m", user_dir);
 
-	if (((rc = stat(hdf5_conf.dir, &st)) < 0) && (errno == ENOENT)) {
-		if (mkdir(hdf5_conf.dir, 0755) < 0)
-			fatal("mkdir(%s): %m", hdf5_conf.dir);
-	} else if (rc < 0)
-		fatal("Unable to stat acct_gather_profile_dir: %s: %m",
-		      hdf5_conf.dir);
-	else if (!S_ISDIR(st.st_mode))
-		fatal("acct_gather_profile_dir: %s: Not a directory!",
-		      hdf5_conf.dir);
-	else if (access(hdf5_conf.dir, R_OK|W_OK|X_OK) < 0)
-		fatal("Incorrect permissions on acct_gather_profile_dir: %s",
-		      hdf5_conf.dir);
-	if (chmod(hdf5_conf.dir, 0755) == -1)
-		error("%s: chmod(%s): %m", __func__, hdf5_conf.dir);
-
-	user_dir = xstrdup_printf("%s/%s", hdf5_conf.dir, g_job->user_name);
-	if (((rc = stat(user_dir, &st)) < 0) && (errno == ENOENT)) {
-		if (mkdir(user_dir, 0700) < 0)
-			fatal("mkdir(%s): %m", user_dir);
+		if (fchownat(user_parent_dirfd, g_job->user_name, g_job->uid,
+			     g_job->gid, AT_SYMLINK_NOFOLLOW) < 0)
+			fatal("fchmodat(%s): %m", user_dir);
 	}
-	if (chmod(user_dir, 0700) == -1)
-		error("%s: chmod(%s): %m", __func__, user_dir);
-	if (chown(user_dir, (uid_t)g_job->uid,
-		  (gid_t)g_job->gid) < 0)
-		error("chown(%s): %m", user_dir);
 
+	close(user_parent_dirfd);
 	xfree(user_dir);
+	xfree(parent_dir);
+	/* Do not xfree() hdf5_dir_rel (interior pointer to freed data). */
 
 	return SLURM_SUCCESS;
 }

Places

File U_13-acct_gather_profile-hdf5-Avoid-TOCTOU-abuse-when-creating-directories.patch of Package slurm

Places