From 186a3ff230373b6f6993c9c131d792ed519be1fd Mon Sep 17 00:00:00 2001
From: Dominik Bartkiewicz <bart@schedmd.com>
Date: Wed, 4 May 2022 13:06:41 -0600
Subject: [PATCH] Prevent abuse of REQUEST_FORWARD_DATA.

CVE-2022-29501
---
 NEWS                         |   2 +
 src/plugins/mpi/pmi2/setup.c |   5 ++
 src/slurmd/slurmd/req.c      | 108 ++++++++++++++++++++++++++++-------
 3 files changed, 93 insertions(+), 22 deletions(-)

diff --git a/NEWS b/NEWS
index 90ccebed87..073e3cbee3 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,8 @@
 This file describes changes in recent versions of Slurm. It primarily
 documents those changes that are of interest to users and administrators.
 
+ -- CVE-2022-29501 - Prevent abuse of REQUEST_FORWARD_DATA.
+
 * Changes in Slurm 17.02.11
 ==========================
  -- Fix insecure handling of user_name and gid fields. CVE-2018-10995.
diff --git a/src/plugins/mpi/pmi2/setup.c b/src/plugins/mpi/pmi2/setup.c
index 21740de104..93884fa211 100644
--- a/src/plugins/mpi/pmi2/setup.c
+++ b/src/plugins/mpi/pmi2/setup.c
@@ -292,6 +292,11 @@ _setup_stepd_sockets(const stepd_step_rec_t *job, char ***env)
 		unlink(sa.sun_path);
 		return SLURM_ERROR;
 	}
+	if (chown(sa.sun_path, job->uid, -1) < 0) {
+		error("mpi/pmi2: failed to chown tree socket: %m");
+		unlink(sa.sun_path);
+		return SLURM_ERROR;
+	}
 	if (listen(tree_sock, 64) < 0) {
 		error("mpi/pmi2: failed to listen tree socket: %m");
 		unlink(sa.sun_path);
diff --git a/src/slurmd/slurmd/req.c b/src/slurmd/slurmd/req.c
index 1d3eadbdb0..37fdc89fca 100644
--- a/src/slurmd/slurmd/req.c
+++ b/src/slurmd/slurmd/req.c
@@ -1655,6 +1655,89 @@ static int _open_as_other(char *path_name, batch_job_launch_msg_t *req)
 	exit(SLURM_SUCCESS);
 }
 
+/*
+ * Connect to unix socket based upon permissions of a different user
+ * IN sock_name - name of socket to open
+ * IN uid - User ID to use for file access check
+ * IN gid - Group ID to use for file access check
+ * OUT fd - File descriptor
+ * RET error or SLURM_SUCCESS
+ * */
+static int _connect_as_other(char *sock_name, uid_t uid, gid_t gid, int *fd)
+{
+	pid_t child;
+	int pipe[2];
+	int rc = 0;
+	struct sockaddr_un sa;
+
+	*fd = -1;
+	if (strlen(sock_name) >= sizeof(sa.sun_path)) {
+		error("%s: Unix socket path '%s' is too long. (%ld > %ld)",
+		      __func__, sock_name,
+		      (long int)(strlen(sock_name) + 1),
+		      (long int)sizeof(sa.sun_path));
+		return EINVAL;
+	}
+
+	/* child process will setuid to the user, register the process
+	 * with the container, and open the file for us. */
+	if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pipe) != 0) {
+		error("%s: Failed to open pipe: %m", __func__);
+		return SLURM_ERROR;
+	}
+
+	child = fork();
+	if (child == -1) {
+		error("%s: fork failure", __func__);
+		close(pipe[0]);
+		close(pipe[1]);
+		return SLURM_ERROR;
+	} else if (child > 0) {
+		int exit_status = -1;
+		close(pipe[0]);
+		(void) waitpid(child, &rc, 0);
+		if (WIFEXITED(rc) && (WEXITSTATUS(rc) == 0))
+			*fd = _receive_fd(pipe[1]);
+		exit_status = WEXITSTATUS(rc);
+		close(pipe[1]);
+		return exit_status;
+	}
+
+	/* child process below here */
+
+	close(pipe[1]);
+
+	if (setgid(gid) < 0) {
+		error("%s: uid:%u setgid(%u): %m", __func__, uid, gid);
+		_exit(errno);
+	}
+	if (setuid(uid) < 0) {
+		error("%s: getuid(%u): %m", __func__, uid);
+		_exit(errno);
+	}
+
+	*fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	if (*fd < 0) {
+		error("%s:failed creating UNIX domain socket: %m", __func__ );
+		_exit(errno);
+	}
+
+	memset(&sa, 0, sizeof(sa));
+	sa.sun_family = AF_UNIX;
+	strcpy(sa.sun_path, sock_name);
+	while (((rc = connect(*fd, (struct sockaddr *)&sa,
+			      SUN_LEN(&sa))) < 0) && (errno == EINTR));
+
+	if (rc < 0) {
+		debug2("%s: failed connecting to specified socket '%s': %m",
+		       __func__, sock_name);
+		_exit(errno);
+	}
+	_send_back_fd(pipe[0], *fd);
+	close(*fd);
+	_exit(SLURM_SUCCESS);
+}
+
 static void
 _prolog_error(batch_job_launch_msg_t *req, int rc)
 {
@@ -6595,8 +6678,8 @@ static void
 _rpc_forward_data(slurm_msg_t *msg)
 {
 	forward_data_msg_t *req = (forward_data_msg_t *)msg->data;
-	uint32_t req_uid;
-	struct sockaddr_un sa;
+        uint32_t req_uid = (uint32_t) g_slurm_auth_get_uid(msg->auth_cred, conf->auth_info);
+	uint32_t req_gid = (uint32_t) g_slurm_auth_get_gid(msg->auth_cred, conf->auth_info);
 	int fd = -1, rc = 0;
 
 	/* Make sure we adjust for the spool dir coming in on the address to
@@ -6607,25 +6690,8 @@ _rpc_forward_data(slurm_msg_t *msg)
 	debug3("Entering _rpc_forward_data, address: %s, len: %u",
 	       req->address, req->len);
 
-	/* sanity check */
-	if (strlen(req->address) > sizeof(sa.sun_path) - 1) {
-		slurm_seterrno(EINVAL);
-		rc = errno;
-		goto done;
-	}
+	rc = _connect_as_other(req->address, req_uid, req_gid, &fd);
 
-	/* connect to specified address */
-	fd = socket(AF_UNIX, SOCK_STREAM, 0);
-	if (fd < 0) {
-		rc = errno;
-		error("failed creating UNIX domain socket: %m");
-		goto done;
-	}
-	memset(&sa, 0, sizeof(sa));
-	sa.sun_family = AF_UNIX;
-	strcpy(sa.sun_path, req->address);
-	while (((rc = connect(fd, (struct sockaddr *)&sa, SUN_LEN(&sa))) < 0) &&
-	       (errno == EINTR));
 	if (rc < 0) {
 		rc = errno;
 		debug2("failed connecting to specified socket '%s': %m",
@@ -6633,8 +6699,6 @@ _rpc_forward_data(slurm_msg_t *msg)
 		goto done;
 	}
 
-	req_uid = (uint32_t)g_slurm_auth_get_uid(msg->auth_cred,
-						 conf->auth_info);
 	/*
 	 * although always in localhost, we still convert it to network
 	 * byte order, to make it consistent with pack/unpack.
-- 
2.35.3

