CVE-2012-4530: stack disclosure binfmt_script load_script (rhbz 868285 880147)

This commit is contained in:
Josh Boyer 2012-11-26 09:03:26 -05:00
parent 0aacb42399
commit 27ddac64c7
3 changed files with 274 additions and 1 deletions

View File

@ -0,0 +1,118 @@
From 20ae2081584450e552735a3df968ce5b5946a607 Mon Sep 17 00:00:00 2001
From: Kees Cook <keescook@chromium.org>
Date: Mon, 26 Nov 2012 08:56:37 -0500
Subject: [PATCH 1/2] exec: do not leave bprm->interp on stack
If a series of scripts are executed, each triggering module loading via
unprintable bytes in the script header, kernel stack contents can leak
into the command line.
Normally execution of binfmt_script and binfmt_misc happens recursively.
However, when modules are enabled, and unprintable bytes exist in the
bprm->buf, execution will restart after attempting to load matching binfmt
modules. Unfortunately, the logic in binfmt_script and binfmt_misc does
not expect to get restarted. They leave bprm->interp pointing to their
local stack. This means on restart bprm->interp is left pointing into
unused stack memory which can then be copied into the userspace argv
areas.
After additional study, it seems that both recursion and restart remains
the desirable way to handle exec with scripts, misc, and modules. As
such, we need to protect the changes to interp.
This changes the logic to require allocation for any changes to the
bprm->interp. To avoid adding a new kmalloc to every exec, the default
value is left as-is. Only when passing through binfmt_script or
binfmt_misc does an allocation take place.
For a proof of concept, see DoTest.sh from:
http://www.halfdog.net/Security/2012/LinuxKernelBinfmtScriptStackDataDisclosure/
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: halfdog <me@halfdog.net>
Cc: P J P <ppandit@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/binfmt_misc.c | 5 ++++-
fs/binfmt_script.c | 4 +++-
fs/exec.c | 15 +++++++++++++++
include/linux/binfmts.h | 1 +
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 790b3cd..772428d 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -176,7 +176,10 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
goto _error;
bprm->argc ++;
- bprm->interp = iname; /* for binfmt_script */
+ /* Update interp in case binfmt_script needs it. */
+ retval = bprm_change_interp(iname, bprm);
+ if (retval < 0)
+ goto _error;
interp_file = open_exec (iname);
retval = PTR_ERR (interp_file);
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index d3b8c1f..df49d48 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -82,7 +82,9 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
retval = copy_strings_kernel(1, &i_name, bprm);
if (retval) return retval;
bprm->argc++;
- bprm->interp = interp;
+ retval = bprm_change_interp(interp, bprm);
+ if (retval < 0)
+ return retval;
/*
* OK, now restart the process with the interpreter's dentry.
diff --git a/fs/exec.c b/fs/exec.c
index fab2c6d..59896ae 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1202,9 +1202,24 @@ void free_bprm(struct linux_binprm *bprm)
mutex_unlock(&current->signal->cred_guard_mutex);
abort_creds(bprm->cred);
}
+ /* If a binfmt changed the interp, free it. */
+ if (bprm->interp != bprm->filename)
+ kfree(bprm->interp);
kfree(bprm);
}
+int bprm_change_interp(char *interp, struct linux_binprm *bprm)
+{
+ /* If a binfmt changed the interp, free it first. */
+ if (bprm->interp != bprm->filename)
+ kfree(bprm->interp);
+ bprm->interp = kstrdup(interp, GFP_KERNEL);
+ if (!bprm->interp)
+ return -ENOMEM;
+ return 0;
+}
+EXPORT_SYMBOL(bprm_change_interp);
+
/*
* install the new credentials for this executable
*/
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 366422b..eb53e15 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -128,6 +128,7 @@ extern int setup_arg_pages(struct linux_binprm * bprm,
unsigned long stack_top,
int executable_stack);
extern int bprm_mm_init(struct linux_binprm *bprm);
+extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);
extern int copy_strings_kernel(int argc, const char *const *argv,
struct linux_binprm *bprm);
extern int prepare_bprm_creds(struct linux_binprm *bprm);
--
1.8.0

View File

@ -0,0 +1,144 @@
From 4ae8186cd77835b45f1b35edb4ce70309287bfc3 Mon Sep 17 00:00:00 2001
From: Kees Cook <keescook@chromium.org>
Date: Mon, 26 Nov 2012 09:02:11 -0500
Subject: [PATCH 2/2] exec: use -ELOOP for max recursion depth
To avoid an explosion of request_module calls on a chain of abusive
scripts, fail maximum recursion with -ELOOP instead of -ENOEXEC. As soon
as maximum recursion depth is hit, the error will fail all the way back
up the chain, aborting immediately.
This also has the side-effect of stopping the user's shell from attempting
to reexecute the top-level file as a shell script. As seen in the
dash source:
if (cmd != path_bshell && errno == ENOEXEC) {
*argv-- = cmd;
*argv = cmd = path_bshell;
goto repeat;
}
The above logic was designed for running scripts automatically that lacked
the "#!" header, not to re-try failed recursion. On a legitimate -ENOEXEC,
things continue to behave as the shell expects.
Additionally, when tracking recursion, the binfmt handlers should not be
involved. The recursion being tracked is the depth of calls through
search_binary_handler(), so that function should be exclusively responsible
for tracking the depth.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: halfdog <me@halfdog.net>
Cc: P J P <ppandit@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
fs/binfmt_em86.c | 1 -
fs/binfmt_misc.c | 6 ------
fs/binfmt_script.c | 4 +---
fs/exec.c | 10 +++++-----
include/linux/binfmts.h | 2 --
5 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index 2790c7e..575796a 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -42,7 +42,6 @@ static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs)
return -ENOEXEC;
}
- bprm->recursion_depth++; /* Well, the bang-shell is implicit... */
allow_write_access(bprm->file);
fput(bprm->file);
bprm->file = NULL;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 772428d..f0f1a06 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -117,10 +117,6 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!enabled)
goto _ret;
- retval = -ENOEXEC;
- if (bprm->recursion_depth > BINPRM_MAX_RECURSION)
- goto _ret;
-
/* to keep locking time low, we copy the interpreter string */
read_lock(&entries_lock);
fmt = check_file(bprm);
@@ -200,8 +196,6 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (retval < 0)
goto _error;
- bprm->recursion_depth++;
-
retval = search_binary_handler (bprm, regs);
if (retval < 0)
goto _error;
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index df49d48..8ae4be1 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -22,15 +22,13 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
char interp[BINPRM_BUF_SIZE];
int retval;
- if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') ||
- (bprm->recursion_depth > BINPRM_MAX_RECURSION))
+ if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!'))
return -ENOEXEC;
/*
* This section does the #! interpretation.
* Sorta complicated, but hopefully it will work. -TYT
*/
- bprm->recursion_depth++;
allow_write_access(bprm->file);
fput(bprm->file);
bprm->file = NULL;
diff --git a/fs/exec.c b/fs/exec.c
index 59896ae..541cc51 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1398,6 +1398,10 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
struct linux_binfmt *fmt;
pid_t old_pid, old_vpid;
+ /* This allows 4 levels of binfmt rewrites before failing hard. */
+ if (depth > 5)
+ return -ELOOP;
+
retval = security_bprm_check(bprm);
if (retval)
return retval;
@@ -1422,12 +1426,8 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
if (!try_module_get(fmt->module))
continue;
read_unlock(&binfmt_lock);
+ bprm->recursion_depth = depth + 1;
retval = fn(bprm, regs);
- /*
- * Restore the depth counter to its starting value
- * in this call, so we don't have to rely on every
- * load_binary function to restore it on return.
- */
bprm->recursion_depth = depth;
if (retval >= 0) {
if (depth == 0) {
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index eb53e15..5bab59b 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -68,8 +68,6 @@ struct linux_binprm {
#define BINPRM_FLAGS_EXECFD_BIT 1
#define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT)
-#define BINPRM_MAX_RECURSION 4
-
/* Function parameter for binfmt->coredump */
struct coredump_params {
long signr;
--
1.8.0

View File

@ -54,7 +54,7 @@ Summary: The Linux kernel
# For non-released -rc kernels, this will be appended after the rcX and
# gitX tags, so a 3 here would become part of release "0.rcX.gitX.3"
#
%global baserelease 4
%global baserelease 5
%global fedora_build %{baserelease}
# base_sublevel is the kernel version we're starting with and patching
@ -727,6 +727,10 @@ Patch22125: Bluetooth-Add-support-for-BCM20702A0.patch
#rhbz CVE-2012-4461 862900 878518
Patch21227: KVM-x86-invalid-opcode-oops-on-SET_SREGS-with-OSXSAV.patch
#rhbz CVE-2012-4530 868285 880147
Patch21228: exec-do-not-leave-bprm-interp-on-stack.patch
Patch21229: exec-use-eloop-for-max-recursion-depth.patch
# END OF PATCH DEFINITIONS
%endif
@ -1371,6 +1375,10 @@ ApplyPatch Bluetooth-Add-support-for-BCM20702A0.patch
#rhbz CVE-2012-4461 862900 878518
ApplyPatch KVM-x86-invalid-opcode-oops-on-SET_SREGS-with-OSXSAV.patch
#rhbz CVE-2012-4530 868285 880147
ApplyPatch exec-do-not-leave-bprm-interp-on-stack.patch
ApplyPatch exec-use-eloop-for-max-recursion-depth.patch
# END OF PATCH APPLICATIONS
%endif
@ -2071,6 +2079,9 @@ fi
# and build.
%changelog
* Mon Nov 26 2012 Josh Boyer <jwboyer@redhat.com>
- CVE-2012-4530: stack disclosure binfmt_script load_script (rhbz 868285 880147)
* Tue Nov 20 2012 Josh Boyer <jwboyer@redhat.com>
- CVE-2012-4461: kvm: invalid opcode oops on SET_SREGS with OSXSAVE bit set (rhbz 878518 862900)
- Add support for BCM20702A0 (rhbz 874791)