[PATCH] Protect execve from being used after 'setuid' exceeds RLIMIT_NPROC

[NeilBrown]

Many programs to not check setuid for failure so it is not safe
for it to fail.  So impose the RLIMIT_NPROC limit by noting the
excess in set_user with a process flag, and failing exec() if the
flag is set.

The patch http://lkml.org/lkml/2003/7/13/226 introduced a RLIMIT_NPROC
check in set_user() to check for NPROC exceeding via setuid() and
similar functions.  Before the check there was a possibility to greatly
exceed the allowed number of processes by an unprivileged user if the
program relied on rlimit only.  But the check created new security
threat: many poorly written programs simply don't check setuid() return
code and believe it cannot fail if executed with root privileges.  So,
when the check fails we set a process flag which causes execve to fail.

Following examples of vulnerabilities due to processes not checking
error from setuid provided by Solar Designer <solar@xxxxxxxxxxxx>:

Here are some examples for 2011-2010:

"... missing setuid() retval check in opielogin which leads to easy root
compromise":

http://www.openwall.com/lists/oss-security/2011/06/22/6

"The /usr/lib/libgnomesu/gnomesu-pam-backend suid binary which belongs
to the libgnomesu package is not checking setuid() return values.

As a result, two cooperating users, or users with access to guest,
cgi or web accounts can run arbitrary commands as root very easily."

http://www.openwall.com/lists/oss-security/2011/05/30/2

pam_xauth (exploitable if pam_limits is also used):

http://www.openwall.com/lists/oss-security/2010/08/16/2

A collection of examples from 2006:

http://lists.openwall.net/linux-kernel/2006/08/20/156

Signed-off-by: NeilBrown <neilb@xxxxxxx>

diff --git a/fs/exec.c b/fs/exec.c
index 6075a1e..dfe9c61 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1434,6 +1434,17 @@ static int do_execve_common(const char *filename,
 	bool clear_in_exec;
 	int retval;
 
+	if (current->flags & PF_NPROC_EXCEEDED) {
+		/* setuid noticed that RLIMIT_NPROC was
+		 * exceeded, but didn't fail as that easily
+		 * leads to privileges leaking.  So fail
+		 * here instead - we still limit the number of
+		 * processes running the user's code.
+		 */
+		retval = -EAGAIN;
+		goto out_ret;
+	}
+
 	retval = unshare_files(&displaced);
 	if (retval)
 		goto out_ret;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 496770a..f024c63 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
 #define PF_DUMPCORE	0x00000200	/* dumped core */
 #define PF_SIGNALED	0x00000400	/* killed by a signal */
 #define PF_MEMALLOC	0x00000800	/* Allocating memory */
+#define PF_NPROC_EXCEEDED 0x00001000	/* set_user noticed that RLIMIT_NPROC was exceeded */
 #define PF_USED_MATH	0x00002000	/* if unset the fpu must be initialized before use */
 #define PF_FREEZING	0x00004000	/* freeze in progress. do not account to load */
 #define PF_NOFREEZE	0x00008000	/* this thread should not be frozen */
diff --git a/kernel/cred.c b/kernel/cred.c
index 174fa84..8ef31f5 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -508,10 +508,8 @@ int commit_creds(struct cred *new)
 		key_fsgid_changed(task);
 
 	/* do it
-	 * - What if a process setreuid()'s and this brings the
-	 *   new uid over his NPROC rlimit?  We can check this now
-	 *   cheaply with the new uid cache, so if it matters
-	 *   we should be checking for it.  -DaveM
+	 * RLIMIT_NPROC limits on user->processes have already been checked
+	 * in set_user().
 	 */
 	alter_cred_subscribers(new, 2);
 	if (new->user != old->user)
diff --git a/kernel/sys.c b/kernel/sys.c
index e4128b2..dd1fb9d 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -592,10 +592,9 @@ static int set_user(struct cred *new)
 		return -EAGAIN;
 
 	if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) &&
-			new_user != INIT_USER) {
-		free_uid(new_user);
-		return -EAGAIN;
-	}
+			new_user != INIT_USER)
+		/* Cause any subsequent 'exec' to fail */
+		current->flags |= PF_NPROC_EXCEEDED;
 
 	free_uid(new->user);
 	new->user = new_user;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/