CVE-2012-4530: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: The load_script function in fs/binfmt_script.c in the Linux kernel before 3.7.2 does not properly handle recursion, which allows local users to obtain sensitive information from kernel stack memory via a crafted application.
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
Published: 2013-02-18T04:41Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: fs/binfmt_misc.c, fs/binfmt_script.c, fs/exec.c, include/linux/binfmts.h
Commit date: 2012-12-20 17:40:19 -0800
Commit message: 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>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Diff: diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 9be335fb8a7cd..0c8869fdd14eb 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -170,11 +170,14 @@ static int load_misc_binary(struct linux_binprm *bprm)
 	retval = copy_strings_kernel (1, &iname_addr, bprm);
 	if (retval < 0)
 		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);
 	if (IS_ERR (interp_file))
 		goto _error;
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 1610a91637e57..5027a3e149222 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -78,11 +78,13 @@ static int load_script(struct linux_binprm *bprm)
 		bprm->argc++;
 	}
 	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.
 	 */
 	file = open_exec(interp);
diff --git a/fs/exec.c b/fs/exec.c
index d8e1191cb112e..237d5342786c4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1173,13 +1173,28 @@ void free_bprm(struct linux_binprm *bprm)
 	free_arg_pages(bprm);
 	if (bprm->cred) {
 		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
  */
 void install_exec_creds(struct linux_binprm *bprm)
 {
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index a4c2b565c835d..bdf3965f0a29a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -110,10 +110,11 @@ extern int suid_dumpable;
 
 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);
 extern void install_exec_creds(struct linux_binprm *bprm);
 extern void set_binfmt(struct linux_binfmt *new);

Vulnerable code: 	bprm->interp = iname;	/* for binfmt_script */
	bprm->interp = interp;
Fixed code: 	/* Update interp in case binfmt_script needs it. */
	retval = bprm_change_interp(iname, bprm);
	if (retval < 0)
		goto _error;
	retval = bprm_change_interp(interp, bprm);
	if (retval < 0)
		return retval;
	/* If a binfmt changed the interp, free it. */
	if (bprm->interp != bprm->filename)
		kfree(bprm->interp);
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);

extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);