[PATCH] Reset current->pdeath_signal on SUID binary execution

[PATCH] Reset current->pdeath_signal on SUID binary execution

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

Hi Linus,

the attached patch fixes a flaw in the "parent process death signal"
when executing SUID binaries. An unprivileged user may send arbitrary
signal to a child process even if it is running with higher privileges.

The idea to fix this issue is to reset pdeath_signal not only on fork,
but also on the execution of a SUID binary.

Michael, if we fix it this way, then the prctl() manual page should
reflect that behavior.

>From comments it seems that we have to also reset pdeath_signal inside
LSM when it comes to capability-raised executes, but I must admit that I
got lost there.

Regards

Marcel


[-- Attachment #2: patch-reset-pdeath-signal-on-suid --]
[-- Type: text/plain, Size: 1726 bytes --]

[PATCH] Reset current->pdeath_signal on SUID binary execution

This fixes a vulnerability in the "parent process death signal"
implementation discoverd by Wojciech Purczynski of COSEINC PTE Ltd.
and iSEC Security Research.

http://marc.info/?l=bugtraq&m=118711306802632&w=2

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>

---
commit 8542f23e44f591480ca53d215481cbec43b8cbed
tree d72f29856adb306f20e828d4bab244685fe24bf2
parent 6adb31c90c47262c8a25bf5097de9b3426caf3ae
author Marcel Holtmann <marcel@holtmann.org> Fri, 17 Aug 2007 21:41:52 +0200
committer Marcel Holtmann <marcel@holtmann.org> Fri, 17 Aug 2007 21:41:52 +0200

 fs/exec.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 7bdea79..ce62f7b 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1084,9 +1084,12 @@ int flush_old_exec(struct linux_binprm * bprm)
 	 */
 	current->mm->task_size = TASK_SIZE;
 
-	if (bprm->e_uid != current->euid || bprm->e_gid != current->egid || 
-	    file_permission(bprm->file, MAY_READ) ||
-	    (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)) {
+	if (bprm->e_uid != current->euid || bprm->e_gid != current->egid) {
+		suid_keys(current);
+		set_dumpable(current->mm, suid_dumpable);
+		current->pdeath_signal = 0;
+	} else if (file_permission(bprm->file, MAY_READ) ||
+			(bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)) {
 		suid_keys(current);
 		set_dumpable(current->mm, suid_dumpable);
 	}
@@ -1177,8 +1180,10 @@ void compute_creds(struct linux_binprm *bprm)
 {
 	int unsafe;
 
-	if (bprm->e_uid != current->uid)
+	if (bprm->e_uid != current->uid) {
 		suid_keys(current);
+		current->pdeath_signal = 0;
+	}
 	exec_keys(current);
 
 	task_lock(current);

Re: [PATCH] Reset current->pdeath_signal on SUID binary execution

[Marcel Holtmann]

Hi,

> the attached patch fixes a flaw in the "parent process death signal"
> when executing SUID binaries. An unprivileged user may send arbitrary
> signal to a child process even if it is running with higher privileges.
> 
> The idea to fix this issue is to reset pdeath_signal not only on fork,
> but also on the execution of a SUID binary.
> 
> Michael, if we fix it this way, then the prctl() manual page should
> reflect that behavior.

the patch has been merged into 2.4 and 2.6, so the manual page needs an
update at some point.

> From comments it seems that we have to also reset pdeath_signal inside
> LSM when it comes to capability-raised executes, but I must admit that I
> got lost there.

No further comments for this one? I am not familiar enough with it.

Regards

Marcel

Re: [PATCH] Reset current->pdeath_signal on SUID binary execution

[Michael Kerrisk]

Marcel,

>> the attached patch fixes a flaw in the "parent process death signal"
>> when executing SUID binaries. An unprivileged user may send arbitrary
>> signal to a child process even if it is running with higher privileges.
>>
>> The idea to fix this issue is to reset pdeath_signal not only on fork,
>> but also on the execution of a SUID binary.
>>
>> Michael, if we fix it this way, then the prctl() manual page should
>> reflect that behavior.
> 
> the patch has been merged into 2.4 and 2.6, so the manual page needs an
> update at some point.

I see your patch at:

http://article.gmane.org/gmane.linux.kernel/571635/match=pdeath%5fsignal+suid+binary+execution

But it's not clear to me in which 2.4.x and 2.6.x versions the change occurred (it isn't in 2.6.23-rc3 -- is it scheduled
for 2.6.23-rc4?).  Can you enlighten me?

Cheers,

Michael

>> From comments it seems that we have to also reset pdeath_signal inside
>> LSM when it comes to capability-raised executes, but I must admit that I
>> got lost there.
> 
> No further comments for this one? I am not familiar enough with it.
> 
> Regards
> 
> Marcel
> 
> 

-- 
Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7

Want to help with man page maintenance?  Grab the latest tarball at
http://www.kernel.org/pub/linux/docs/manpages/
read the HOWTOHELP file and grep the source files for 'FIXME'.

Re: [PATCH] Reset current->pdeath_signal on SUID binary execution

[Linus Torvalds]

On Mon, 27 Aug 2007, Michael Kerrisk wrote:
> 
> But it's not clear to me in which 2.4.x and 2.6.x versions the change 
> occurred (it isn't in 2.6.23-rc3 -- is it scheduled for 2.6.23-rc4?).  

It's in current -git (commit d2d56c5f51028cb9f3d800882eb6f4cbd3f9099f), so 
yes, it will be in -rc4 (which should happen today).

		Linus

Re: [PATCH] Reset current->pdeath_signal on SUID binary execution

[Michael Kerrisk (man-pages)]

Marcel,

On Sat, Aug 18, 2007 at 7:47 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Linus,
>
> the attached patch fixes a flaw in the "parent process death signal"
> when executing SUID binaries. An unprivileged user may send arbitrary
> signal to a child process even if it is running with higher privileges.
>
> The idea to fix this issue is to reset pdeath_signal not only on fork,
> but also on the execution of a SUID binary.
>
> Michael, if we fix it this way, then the prctl() manual page should
> reflect that behavior.

For man-pages-3.40, I applied the patch below.

Cheers,

Michael

--- a/man2/prctl.2
+++ b/man2/prctl.2
@@ -220,7 +220,9 @@ in the range 1..maxsig, or 0 to clear).
 This is the signal that the calling process will get when its
 parent dies.
 This value is cleared for the child of a
-.BR fork (2).
+.BR fork (2)
+and (since Linux 2.5.36 / 2.6.23)
+when execing a set-user_ID or set-group-ID binary.
 .TP
 .BR PR_GET_PDEATHSIG " (since Linux 2.3.15)"
 Return the current value of the parent process death signal,


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/