[prev in list] [next in list] [prev in thread] [next in thread] 

List:       oss-security
Subject:    [oss-security] Linux kernel: fs: fix get_dumpable() incorrect tests (CVE-2013-2929)
From:       Solar Designer <solar () openwall ! com>
Date:       2014-01-31 0:51:55
Message-ID: 20140131005155.GA17396 () openwall ! com
[Download RAW message or body]

I'm afraid the issue below was never brought to oss-security (as it must
have been).  The fix was committed on November 13:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d049f74f2dbe71354d43d393ac3a188947811348


including detailed description and the CVE-2013-2929 reference.  So it
was clearly disclosed as a security issue, yet bringing it to
oss-security specifically seems to have falled through the cracks. :-(

Kees' posting to linux-distros:

----- Forwarded message from Kees Cook <kees@outflux.net> -----

From: Kees Cook <kees@outflux.net>
Subject: [PATCH] fs: fix get_dumpable() incorrect tests
Date: Tue, 22 Oct 2013 16:25:30 -0700

The get_dumpable() return value is not boolean. Most users of the
function actually want to be testing for SUID_DUMP_USER rather than
non-zero.

Without this patch, if the system had set the sysctl fs/suid_dumpable=2,
a user was able to ptrace attach to processes that had dropped privileges
to that user. (This may have been partially mitigated if Yama was enabled.)

CVE-2013-2929

Reported-by: Vasily Kulikov <segoon@openwall.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
---
I plan to send this to security@kernel.org on Monday, unless anyone
would like to push that a bit. Also note, I do not have ia64 cross
compilers available, so I used the "1" literal instead of adding
binfmts.h to the ia64 code.
---
 arch/ia64/include/asm/processor.h |    2 +-
 fs/exec.c                         |    6 ++++++
 kernel/ptrace.c                   |    4 +++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/include/asm/processor.h b/arch/ia64/include/asm/processor.h
index e0a899a..d036a33 100644
--- a/arch/ia64/include/asm/processor.h
+++ b/arch/ia64/include/asm/processor.h
@@ -319,7 +319,7 @@ struct thread_struct {
 	regs->loadrs = 0;									\
 	regs->r8 = get_dumpable(current->mm);	/* set "don't zap registers" flag */		\
 	regs->r12 = new_sp - 16;	/* allocate 16 byte scratch area */			\
-	if (unlikely(!get_dumpable(current->mm))) {							\
+	if (unlikely(get_dumpable(current->mm) != 1)) {	\
 		/*										\
 		 * Zap scratch regs to avoid leaking bits between processes with different	\
 		 * uid/privileges.								\
diff --git a/fs/exec.c b/fs/exec.c
index 8875dd1..bb8afc1 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1668,6 +1668,12 @@ int __get_dumpable(unsigned long mm_flags)
 	return (ret > SUID_DUMP_USER) ? SUID_DUMP_ROOT : ret;
 }
 
+/*
+ * This returns the actual value of the suid_dumpable flag. For things
+ * that are using this for checking for privilege transitions, it must
+ * test against SUID_DUMP_USER rather than treating it as a boolean
+ * value.
+ */
 int get_dumpable(struct mm_struct *mm)
 {
 	return __get_dumpable(mm->flags);
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index dd562e9..a75ad41 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -26,6 +26,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/cn_proc.h>
 #include <linux/compat.h>
+#include <linux/binfmts.h>
 
 
 static int ptrace_trapping_sleep_fn(void *flags)
@@ -257,7 +258,8 @@ ok:
 	if (task->mm)
 		dumpable = get_dumpable(task->mm);
 	rcu_read_lock();
-	if (!dumpable && !ptrace_has_cap(__task_cred(task)->user_ns, mode)) {
+	if (dumpable != SUID_DUMP_USER &&
+	    !ptrace_has_cap(__task_cred(task)->user_ns, mode)) {
 		rcu_read_unlock();
 		return -EPERM;
 	}
-- 
1.7.9.5


-- 
Kees Cook                                            @outflux.net

----- End forwarded message -----


[prev in list] [next in list] [prev in thread] [next in thread]