CVE-2016-10030: Improper Access Control (CWE-284)
value: The _prolog_error function in slurmd/req.c in Slurm before 15.08.13, 16.x before 16.05.7, and 17.x before 17.02.0-pre4 has a vulnerability in how the slurmd daemon informs users of a Prolog failure on a compute node. That vulnerability could allow a user to assume control of an arbitrary file on the system. Any exploitation of this is dependent on the user being able to cause or anticipate the failure (non-zero return code) of a Prolog script that their job would run on. This issue affects all Slurm versions from 0.6.0 (September 2005) to present. Workarounds to prevent exploitation of this are to either disable your Prolog script, or modify it such that it always returns 0 ("success") and adjust it to set the node as down using scontrol instead of relying on the slurmd to handle that automatically. If you do not have a Prolog set you are unaffected by this issue.
CWE description: The product does not restrict or incorrectly restricts access to a resource from an unauthorized actor.
CVSS v3 base score: 8.1
Published: 2017-01-05T11:59Z
Repository: https://github.com/SchedMD/slurm
Language: Other
Files changed: NEWS, src/slurmd/slurmd/req.c
Commit date: 2017-01-04 14:10:01 -0700
Commit message: Fix security issue in _prolog_error().

Fix security issue caused by insecure file path handling triggered by
the failure of a Prolog script. To exploit this a user needs to
anticipate or cause the Prolog to fail for their job.

(This commit is slightly different from the fix to the 15.08 branch.)

CVE-2016-10030.

Diff: diff --git a/NEWS b/NEWS
index bca67ea2f6..79a081cfa7 100644
--- a/NEWS
+++ b/NEWS
@@ -26,10 +26,13 @@ documents those changes that are of interest to users and administrators.
     the salloc or srun command as needed.
  -- Prevent possible out-of-bounds read in slurmstepd on an invalid #! line.
  -- Fix check for PluginDir within slurmctld to work with multiple directories.
  -- Cancel interactive jobs automatically on communication error to launching
     srun/salloc process.
+ -- Fix security issue caused by insecure file path handling triggered by the
+    failure of a Prolog script. To exploit this a user needs to anticipate or
+    cause the Prolog to fail for their job. CVE-2016-10030.
 
 * Changes in Slurm 16.05.7
 ==========================
  -- Fix issue in the priority/multifactor plugin where on a slurmctld restart,
     where more time is accounted for than should be allowed.
diff --git a/src/slurmd/slurmd/req.c b/src/slurmd/slurmd/req.c
index 20e6698bdb..477efa3e5c 100644
--- a/src/slurmd/slurmd/req.c
+++ b/src/slurmd/slurmd/req.c
@@ -166,10 +166,11 @@ static int  _launch_job_fail(uint32_t job_id, uint32_t slurm_rc);
 static bool _launch_job_test(uint32_t job_id);
 static void _note_batch_job_finished(uint32_t job_id);
 static int  _prolog_is_running (uint32_t jobid);
 static int  _step_limits_match(void *x, void *key);
 static int  _terminate_all_steps(uint32_t jobid, bool batch);
+static int  _receive_fd(int socket);
 static void _rpc_launch_tasks(slurm_msg_t *);
 static void _rpc_abort_job(slurm_msg_t *);
 static void _rpc_batch_job(slurm_msg_t *msg, bool new_msg);
 static void _rpc_prolog(slurm_msg_t *msg);
 static void _rpc_job_notify(slurm_msg_t *);
@@ -212,10 +213,11 @@ static bool _pause_for_job_completion(uint32_t jobid, char *nodes,
 static bool _slurm_authorized_user(uid_t uid);
 static void _sync_messages_kill(kill_job_msg_t *req);
 static int  _waiter_init (uint32_t jobid);
 static int  _waiter_complete (uint32_t jobid);
 
+static void _send_back_fd(int socket, int fd);
 static bool _steps_completed_now(uint32_t jobid);
 static int  _valid_sbcast_cred(file_bcast_msg_t *req, uid_t req_uid,
 			       uint16_t block_no, uint32_t *job_id);
 static void _wait_state_completed(uint32_t jobid, int max_delay);
 static uid_t _get_job_uid(uint32_t jobid);
@@ -1381,10 +1383,115 @@ _rpc_launch_tasks(slurm_msg_t *msg)
 	 */
 	if (errnum == ESLURMD_PROLOG_FAILED)
 		send_registration_msg(errnum, false);
 }
 
+/*
+ * Open file based upon permissions of a different user
+ * IN path_name - name of file to open
+ * IN uid - User ID to use for file access check
+ * IN gid - Group ID to use for file access check
+ * RET -1 on error, file descriptor otherwise
+ */
+static int _open_as_other(char *path_name, batch_job_launch_msg_t *req)
+{
+	pid_t child;
+	gids_t *gids;
+	int pipe[2];
+	int fd = -1, rc = 0;
+
+	if (!(gids = _gids_cache_lookup(req->user_name, req->gid))) {
+		error("%s: gids_cache_lookup for %s failed",
+		      __func__, req->user_name);
+		return -1;
+	}
+
+	if ((rc = container_g_create(req->job_id))) {
+		error("%s: container_g_create(%u): %m", __func__, req->job_id);
+		_dealloc_gids(gids);
+		return -1;
+	}
+
+	/* 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__);
+		_dealloc_gids(gids);
+		return -1;
+	}
+
+	child = fork();
+	if (child == -1) {
+		error("%s: fork failure", __func__);
+		_dealloc_gids(gids);
+		close(pipe[0]);
+		close(pipe[1]);
+		return -1;
+	} else if (child > 0) {
+		close(pipe[0]);
+		(void) waitpid(child, &rc, 0);
+		_dealloc_gids(gids);
+		if (WIFEXITED(rc) && (WEXITSTATUS(rc) == 0))
+			fd = _receive_fd(pipe[1]);
+		close(pipe[1]);
+		return fd;
+	}
+
+	/* child process below here */
+
+	close(pipe[1]);
+
+	/* container_g_add_pid needs to be called in the
+	 * forked process part of the fork to avoid a race
+	 * condition where if this process makes a file or
+	 * detacts itself from a child before we add the pid
+	 * to the container in the parent of the fork. */
+	if (container_g_add_pid(req->job_id, getpid(), req->uid)) {
+		error("%s container_g_add_pid(%u): %m", __func__, req->job_id);
+		exit(SLURM_ERROR);
+	}
+
+	/* The child actually performs the I/O and exits with
+	 * a return code, do not return! */
+
+	/*********************************************************************\
+	 * NOTE: It would be best to do an exec() immediately after the fork()
+	 * in order to help prevent a possible deadlock in the child process
+	 * due to locks being set at the time of the fork and being freed by
+	 * the parent process, but not freed by the child process. Performing
+	 * the work inline is done for simplicity. Note that the logging
+	 * performed by error() should be safe due to the use of
+	 * atfork_install_handlers() as defined in src/common/log.c.
+	 * Change the code below with caution.
+	\*********************************************************************/
+
+	if (setgroups(gids->ngids, gids->gids) < 0) {
+		error("%s: uid: %u setgroups failed: %m", __func__, req->uid);
+		exit(errno);
+	}
+	_dealloc_gids(gids);
+
+	if (setgid(req->gid) < 0) {
+		error("%s: uid:%u setgid(%u): %m", __func__, req->uid,req->gid);
+		exit(errno);
+	}
+	if (setuid(req->uid) < 0) {
+		error("%s: getuid(%u): %m", __func__, req->uid);
+		exit(errno);
+	}
+
+	fd = open(path_name, (O_CREAT|O_APPEND|O_WRONLY), 0644);
+	if (fd == -1) {
+		error("%s: uid:%u can't open `%s`: %m",
+		      __func__, req->uid, path_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)
 {
 	char *err_name_ptr, err_name[256], path_name[MAXPATHLEN];
 	char *fmt_char;
@@ -1413,14 +1520,12 @@ _prolog_error(batch_job_launch_msg_t *req, int rc)
 	else if (req->work_dir)
 		snprintf(path_name, MAXPATHLEN, "%s/%s",
 			req->work_dir, err_name_ptr);
 	else
 		snprintf(path_name, MAXPATHLEN, "/%s", err_name_ptr);
-
-	if ((fd = open(path_name, (O_CREAT|O_APPEND|O_WRONLY), 0644)) == -1) {
-		error("Unable to open %s: %s", path_name,
-		      slurm_strerror(errno));
+	if ((fd = _open_as_other(path_name, req)) == -1) {
+		error("Unable to open %s: Permission denied", path_name);
 		return;
 	}
 	snprintf(err_name, sizeof(err_name),
 		 "Error running slurm prolog: %d\n", WEXITSTATUS(rc));
 	safe_write(fd, err_name, strlen(err_name));

Vulnerable code: 
	if ((fd = open(path_name, (O_CREAT|O_APPEND|O_WRONLY), 0644)) == -1) {
		error("Unable to open %s: %s", path_name,
		      slurm_strerror(errno));
Fixed code:  -- Fix security issue caused by insecure file path handling triggered by the
    failure of a Prolog script. To exploit this a user needs to anticipate or
    cause the Prolog to fail for their job. CVE-2016-10030.
static int  _receive_fd(int socket);
static void _send_back_fd(int socket, int fd);
/*
 * Open file based upon permissions of a different user
 * IN path_name - name of file to open
 * IN uid - User ID to use for file access check
 * IN gid - Group ID to use for file access check
 * RET -1 on error, file descriptor otherwise
 */
static int _open_as_other(char *path_name, batch_job_launch_msg_t *req)
{
	pid_t child;
	gids_t *gids;
	int pipe[2];
	int fd = -1, rc = 0;

	if (!(gids = _gids_cache_lookup(req->user_name, req->gid))) {
		error("%s: gids_cache_lookup for %s failed",
		      __func__, req->user_name);
		return -1;
	}

	if ((rc = container_g_create(req->job_id))) {
		error("%s: container_g_create(%u): %m", __func__, req->job_id);
		_dealloc_gids(gids);
		return -1;
	}

	/* 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__);
		_dealloc_gids(gids);
		return -1;
	}

	child = fork();
	if (child == -1) {
		error("%s: fork failure", __func__);
		_dealloc_gids(gids);
		close(pipe[0]);
		close(pipe[1]);
		return -1;
	} else if (child > 0) {
		close(pipe[0]);
		(void) waitpid(child, &rc, 0);
		_dealloc_gids(gids);
		if (WIFEXITED(rc) && (WEXITSTATUS(rc) == 0))
			fd = _receive_fd(pipe[1]);
		close(pipe[1]);
		return fd;
	}

	/* child process below here */

	close(pipe[1]);

	/* container_g_add_pid needs to be called in the
	 * forked process part of the fork to avoid a race
	 * condition where if this process makes a file or
	 * detacts itself from a child before we add the pid
	 * to the container in the parent of the fork. */
	if (container_g_add_pid(req->job_id, getpid(), req->uid)) {
		error("%s container_g_add_pid(%u): %m", __func__, req->job_id);
		exit(SLURM_ERROR);
	}

	/* The child actually performs the I/O and exits with
	 * a return code, do not return! */

	/*********************************************************************\
	 * NOTE: It would be best to do an exec() immediately after the fork()
	 * in order to help prevent a possible deadlock in the child process
	 * due to locks being set at the time of the fork and being freed by
	 * the parent process, but not freed by the child process. Performing
	 * the work inline is done for simplicity. Note that the logging
	 * performed by error() should be safe due to the use of
	 * atfork_install_handlers() as defined in src/common/log.c.
	 * Change the code below with caution.
	\*********************************************************************/

	if (setgroups(gids->ngids, gids->gids) < 0) {
		error("%s: uid: %u setgroups failed: %m", __func__, req->uid);
		exit(errno);
	}
	_dealloc_gids(gids);

	if (setgid(req->gid) < 0) {
		error("%s: uid:%u setgid(%u): %m", __func__, req->uid,req->gid);
		exit(errno);
	}
	if (setuid(req->uid) < 0) {
		error("%s: getuid(%u): %m", __func__, req->uid);
		exit(errno);
	}

	fd = open(path_name, (O_CREAT|O_APPEND|O_WRONLY), 0644);
	if (fd == -1) {
		error("%s: uid:%u can't open `%s`: %m",
		      __func__, req->uid, path_name);
		exit(errno);
	}
	_send_back_fd(pipe[0], fd);
	close(fd);
	exit(SLURM_SUCCESS);
}

	if ((fd = _open_as_other(path_name, req)) == -1) {
		error("Unable to open %s: Permission denied", path_name);