* [PATCH] Prevent setting 32 uids/gids in the error range
@ 2003-02-17 7:41 Rusty Russell
2003-02-17 14:55 ` Alan Cox
0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2003-02-17 7:41 UTC (permalink / raw)
To: torvalds; +Cc: tridge, linux-kernel, cyeoh, sfr
Tridge noticed that getegid() was returning EPERM.
I used -1000 since that's what PTR_ERR uses, but i386 _syscall macros
use -125: I don't suppose it really matters.
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
Name: User and Group ID range check patch
Author: Rusty Russell
Status: Tested on 2.5.61-bk1
D: sys_getuid etc return long, which means that setting your gid to -1
D: on 32-bit platforms causes sys_getuid to return "EPERM", for example.
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.61-bk1/include/linux/types.h working-2.5.61-bk1-valid_ids/include/linux/types.h
--- linux-2.5.61-bk1/include/linux/types.h 2003-02-11 14:26:19.000000000 +1100
+++ working-2.5.61-bk1-valid_ids/include/linux/types.h 2003-02-17 17:26:38.000000000 +1100
@@ -34,6 +34,10 @@ typedef __kernel_gid32_t gid_t;
typedef __kernel_uid16_t uid16_t;
typedef __kernel_gid16_t gid16_t;
+/* If we allowed these to be set, getuid etc. would break (they return long) */
+#define is_valid_gid(gid) ((unsigned long)(gid) < -1000UL)
+#define is_valid_uid(uid) ((unsigned long)(uid) < -1000UL)
+
#ifdef CONFIG_UID16
/* This is defined by include/asm-{arch}/posix_types.h */
typedef __kernel_old_uid_t old_uid_t;
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.61-bk1/kernel/sys.c working-2.5.61-bk1-valid_ids/kernel/sys.c
--- linux-2.5.61-bk1/kernel/sys.c 2003-02-17 11:37:55.000000000 +1100
+++ working-2.5.61-bk1-valid_ids/kernel/sys.c 2003-02-17 18:11:01.000000000 +1100
@@ -658,6 +658,9 @@ asmlinkage long sys_setuid(uid_t uid)
if (retval)
return retval;
+ if (!is_valid_uid(uid))
+ return -EINVAL;
+
old_ruid = new_ruid = current->uid;
old_suid = current->suid;
new_suid = old_suid;
@@ -700,6 +705,11 @@ asmlinkage long sys_setresuid(uid_t ruid
if (retval)
return retval;
+ if ((!is_valid_uid(ruid) && ruid != (uid_t)-1)
+ || (!is_valid_uid(euid) && euid != (uid_t)-1)
+ || (!is_valid_uid(suid) && suid != (uid_t)-1))
+ return -EINVAL;
+
if (!capable(CAP_SETUID)) {
if ((ruid != (uid_t) -1) && (ruid != current->uid) &&
(ruid != current->euid) && (ruid != current->suid))
@@ -752,6 +762,11 @@ asmlinkage long sys_setresgid(gid_t rgid
if (retval)
return retval;
+ if ((!is_valid_gid(rgid) && rgid != (gid_t)-1)
+ || (!is_valid_gid(egid) && egid != (gid_t)-1)
+ || (!is_valid_gid(sgid) && sgid != (gid_t)-1))
+ return -EINVAL;
+
if (!capable(CAP_SETGID)) {
if ((rgid != (gid_t) -1) && (rgid != current->gid) &&
(rgid != current->egid) && (rgid != current->sgid))
@@ -806,6 +821,9 @@ asmlinkage long sys_setfsuid(uid_t uid)
if (retval)
return retval;
+ if (!is_valid_uid(uid))
+ return -EINVAL;
+
old_fsuid = current->fsuid;
if (uid == current->uid || uid == current->euid ||
uid == current->suid || uid == current->fsuid ||
@@ -838,6 +856,9 @@ asmlinkage long sys_setfsgid(gid_t gid)
if (retval)
return retval;
+ if (!is_valid_gid(gid))
+ return -EINVAL;
+
old_fsgid = current->fsgid;
if (gid == current->gid || gid == current->egid ||
gid == current->sgid || gid == current->fsgid ||
@@ -1059,7 +1080,7 @@ asmlinkage long sys_getgroups(int gidset
asmlinkage long sys_setgroups(int gidsetsize, gid_t *grouplist)
{
gid_t groups[NGROUPS];
- int retval;
+ int retval, i;
if (!capable(CAP_SETGID))
return -EPERM;
@@ -1067,6 +1088,9 @@ asmlinkage long sys_setgroups(int gidset
return -EINVAL;
if(copy_from_user(groups, grouplist, gidsetsize * sizeof(gid_t)))
return -EFAULT;
+ for (i = 0; i < gidsetsize; i++)
+ if (!is_valid_gid(groups[i]))
+ return -EINVAL;
retval = security_task_setgroups(gidsetsize, groups);
if (retval)
return retval;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Prevent setting 32 uids/gids in the error range
2003-02-17 7:41 [PATCH] Prevent setting 32 uids/gids in the error range Rusty Russell
@ 2003-02-17 14:55 ` Alan Cox
2003-02-17 15:05 ` Alan Cox
2003-02-18 4:01 ` Rusty Russell
0 siblings, 2 replies; 5+ messages in thread
From: Alan Cox @ 2003-02-17 14:55 UTC (permalink / raw)
To: Rusty Russell
Cc: Linus Torvalds, tridge, Linux Kernel Mailing List, cyeoh, sfr
On Mon, 2003-02-17 at 07:41, Rusty Russell wrote:
> Tridge noticed that getegid() was returning EPERM.
>
> I used -1000 since that's what PTR_ERR uses, but i386 _syscall macros
> use -125: I don't suppose it really matters.
Thats a bug in the interface. getegid/getgid/setegid/setuid() is not permitted to fail.
If libc is setting errno and returning -1 the libc wrapper is wrong.
set*id can fail, but unlike the get functions they do not return a uid but an error code.
Alan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Prevent setting 32 uids/gids in the error range
2003-02-17 14:55 ` Alan Cox
@ 2003-02-17 15:05 ` Alan Cox
2003-02-18 4:01 ` Rusty Russell
1 sibling, 0 replies; 5+ messages in thread
From: Alan Cox @ 2003-02-17 15:05 UTC (permalink / raw)
To: Rusty Russell
Cc: Linus Torvalds, tridge, Linux Kernel Mailing List, cyeoh, sfr
On Mon, 2003-02-17 at 14:55, Alan Cox wrote:
> On Mon, 2003-02-17 at 07:41, Rusty Russell wrote:
> > Tridge noticed that getegid() was returning EPERM.
> >
> > I used -1000 since that's what PTR_ERR uses, but i386 _syscall macros
> > use -125: I don't suppose it really matters.
>
> Thats a bug in the interface. getegid/getgid/setegid/setuid() is not permitted to fail.
I meant geteuid/getuid not set* - set* can fail.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Prevent setting 32 uids/gids in the error range
2003-02-17 14:55 ` Alan Cox
2003-02-17 15:05 ` Alan Cox
@ 2003-02-18 4:01 ` Rusty Russell
2003-02-18 5:02 ` tridge
1 sibling, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2003-02-18 4:01 UTC (permalink / raw)
To: Alan Cox
Cc: Linus Torvalds, tridge, Linux Kernel Mailing List, cyeoh, sfr,
anton, paulus, drepper
In message <1045493720.19397.4.camel@irongate.swansea.linux.org.uk> you write:
> On Mon, 2003-02-17 at 07:41, Rusty Russell wrote:
> > Tridge noticed that getegid() was returning EPERM.
> >
> > I used -1000 since that's what PTR_ERR uses, but i386 _syscall macros
> > use -125: I don't suppose it really matters.
>
> Thats a bug in the interface. getegid/getgid/setegid/setuid() is not permitted to fail.
> If libc is setting errno and returning -1 the libc wrapper is wrong.
OK, thanks. Note that out asm-i386/unistd.h syscallX macros do it
wrong, too.
AFAICT, glibc (2.3.1) gets this wrong, and interprets 0xffffffff
return from geteuid() as an error (ie. it's not just an strace bug).
Tested by Tridge.
Paulus points out that this also means special handling on archs (PPC
and PPC64 that I know of) which set a condition code on error: this
can be sidestepped in glibc, as well.
Other archs beware.
Thanks for the clarification,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Prevent setting 32 uids/gids in the error range
2003-02-18 4:01 ` Rusty Russell
@ 2003-02-18 5:02 ` tridge
0 siblings, 0 replies; 5+ messages in thread
From: tridge @ 2003-02-18 5:02 UTC (permalink / raw)
To: rusty; +Cc: alan, torvalds, linux-kernel, cyeoh, sfr, anton, paulus, drepper
> AFAICT, glibc (2.3.1) gets this wrong, and interprets 0xffffffff
> return from geteuid() as an error (ie. it's not just an strace bug).
> Tested by Tridge.
In case anyone wants it, a simple test program is at
http://samba.org/ftp/unpacked/junkcode/getegid.c
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-02-18 4:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-02-17 7:41 [PATCH] Prevent setting 32 uids/gids in the error range Rusty Russell
2003-02-17 14:55 ` Alan Cox
2003-02-17 15:05 ` Alan Cox
2003-02-18 4:01 ` Rusty Russell
2003-02-18 5:02 ` tridge
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®