Hi, When calling getresuid() from a setuid program, ruid and uid are exchanged. This can make setuid program set the effective uid as real uid too, so that the user that launched the program can't kill it any more... I'll attach an obvious fix. Regards, Samuel
Created attachment 863 [details] fix order of parameters of hurd's getresuid()
I'll attach a complete patch.
Created attachment 866 [details] [rs]etres[gu]id.patch
Please apply [rs]etres[gu]id.patch to both the trunk and glibc-2_3-branch.
Do you have a small testcase to demonstrate the problem?
Created attachment 867 [details] Testcase for getresuid Fails on the Hurd. Fixed by attached patch.
Comment on attachment 867 [details] Testcase for getresuid I forgot to say: set the binary setuid root. The output will then be something like 1000 0 0 1000 0 while it should be 1000 0 1000 0 0
(In reply to comment #5) > Do you have a small testcase to demonstrate the problem? Just have a look at the corresponding Linux code, which is where these functions come from as I understand it. [glibc]/sysdeps/unix/sysv/linux/i386/getresuid.c #v+ [...] int __getresuid (uid_t *ruid, uid_t *euid, uid_t *suid) { # if __ASSUME_32BITUIDS > 0 return INLINE_SYSCALL (getresuid32, 3, CHECK_1 (ruid), CHECK_1 (euid), CHECK_1 (suid)); # else [...] result = INLINE_SYSCALL (getresuid, 3, __ptrvalue (&k_ruid), __ptrvalue (&k_euid), __ptrvalue (&k_suid)); if (result == 0) { *ruid = (uid_t) k_ruid; *euid = (uid_t) k_euid; *suid = (uid_t) k_suid; } return result; # endif } [...] #v- The syscall looks like this: [linux]/kernel/sys.c #v+ [...] asmlinkage long sys_getresuid(uid_t __user *ruid, uid_t __user *euid, uid_t __user *suid) { int retval; if (!(retval = put_user(current->uid, ruid)) && !(retval = put_user(current->euid, euid))) retval = put_user(current->suid, suid); return retval; } [...] #v-
2decimal: you need GNU Hurd to reproduce the bug. This is a grave security issue on hurd, glibc on other kernels seems to be unaffected.
Subject: Bug 2329 CVSROOT: /cvs/glibc Module name: libc Changes by: roland@sources.redhat.com 2006-02-21 02:28:30 Modified files: posix : unistd.h getresgid.c setresuid.c getresuid.c setresgid.c sysdeps/mach/hurd: getresgid.c setresuid.c getresuid.c setresgid.c include : unistd.h Log message: 2006-02-15 Thomas Schwinge <tschwinge@gnu.org> [BZ #2329] * include/unistd.h (__getresuid, __getresgid, __setresuid) (__setresgid): Fix argument name order in prototypes. * posix/unistd.h (getresuid, getresgid, setresuid, setresgid): Likewise. * posix/getresuid.c (__getresuid): Fix argument order in definition. * posix/getresgid.c (__getresgid): Likewise. * posix/setresuid.c (__setresuid): Likewise. * posix/setresgid.c (__setresgid): Likewise. * sysdeps/mach/hurd/getresuid.c (__getresuid): Likewise. * sysdeps/mach/hurd/getresgid.c (__getresgid): Likewise. * sysdeps/mach/hurd/setresuid.c (__setresuid): Likewise. * sysdeps/mach/hurd/setresgid.c (__setresgid): Likewise. Reported by Samuel Thibault <samuel.thibault@ens-lyon.org>. Patches: http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/posix/unistd.h.diff?cvsroot=glibc&r1=1.143&r2=1.144 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/posix/getresgid.c.diff?cvsroot=glibc&r1=1.1&r2=1.2 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/posix/setresuid.c.diff?cvsroot=glibc&r1=1.1&r2=1.2 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/posix/getresuid.c.diff?cvsroot=glibc&r1=1.1&r2=1.2 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/posix/setresgid.c.diff?cvsroot=glibc&r1=1.1&r2=1.2 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/mach/hurd/getresgid.c.diff?cvsroot=glibc&r1=1.1&r2=1.2 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/mach/hurd/setresuid.c.diff?cvsroot=glibc&r1=1.2&r2=1.3 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/mach/hurd/getresuid.c.diff?cvsroot=glibc&r1=1.1&r2=1.2 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/sysdeps/mach/hurd/setresgid.c.diff?cvsroot=glibc&r1=1.2&r2=1.3 http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/include/unistd.h.diff?cvsroot=glibc&r1=1.45&r2=1.46
Committed on the trunk. Needed on 2.3 as well.
Fixed in head, 2.3 doesn't really matter these days.
This is subtle enough that it could lead to security issues in applications. But it is Hurd-specific.