public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Setreuid distinction about (uid_t)-1
@ 2012-07-17  9:27 Adrián
  2012-07-17 13:52 ` Athanasius
  0 siblings, 1 reply; 5+ messages in thread
From: Adrián @ 2012-07-17  9:27 UTC (permalink / raw)
  To: linux-kernel

Hi folks,

first I'd like to apologize if the question I'm asking is dumb or a
little bit out of the scope of the list. I've been doing some testing
on setuid functions family lately, and I found a weird behaviour I'm
not able to explain myself. I'm using this small program to try and
switch the uid of a user:

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/types.h>

int main(int argc, char** argv){
        unsigned int uid;
        char *args[] = {"/bin/sh",NULL};

        if (argc < 2){
                printf("Usage: %s target_uid\n", argv[0]);
                exit(0);
        }
        uid = atoi(argv[1]);
        printf("%u\n",uid);
        if (setreuid(uid,uid)==-1){
                printf("Setreuid to %u failed\n ",uid);
                perror("E");
                exit(1);
        }
        execve("/bin/sh",args,NULL);
        return 1;
}

I've been calling this binary with a bunch of different uid numbers,
and I came across this weird behaviour with the (uid_t) -1 value:

adrian@home-pc:~$ /tmp/suid-tests
Usage: /tmp/suid-tests target_uid
adrian@home-pc:~$ /tmp/suid-tests 0
0
Setreuid to 0 failed
E: Operation not permitted
adrian@home-pc:~$ /tmp/suid-tests -1
4294967295
$ id
uid=1000(adrian) gid=1000(adrian)
groups=1000(adrian),4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin)
adrian@home-pc:~$ /tmp/suid-tests -2
4294967294
Setreuid to 4294967294 failed
E: Operation not permitted
adrian@home-pc:~$ /tmp/suid-tests -3
4294967293
Setreuid to 4294967293 failed
E: Operation not permitted

If the binary is setuid, the -1 call effectively rises the euid to
root (0), although other arbitrary values are properly being set:

adrian@home-pc:~$ ls -hl /tmp/suid-tests
-rwsr-x--- 1 root adrian 8,5K 2012-07-17 10:53 /tmp/suid-tests
adrian@home-pc:~$ /tmp/suid-tests -1
4294967295
# id
uid=1000(adrian) gid=1000(adrian) euid=0(root)
groups=0(root),4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin),1000(adrian)
adrian@home-pc:~$ /tmp/suid-tests -2
4294967294
$ id
uid=4294967294 gid=1000(adrian)
groups=4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin),1000(adrian)


I've been looking into kernel/sys.c, reading the setreuid function for
an explanation. I've seen that there are several if cases for when the
uid value is (uid_t)-1 but I still don't understand why is this being
doing. I tried to trace down all the checks that take place, but I'm
not quite familiar with the kernel and I feel I'm missing something.
Is this an expected behaviour? If so, could someone please shed some
light on why?

Running kernels for the tests have been several on the 2.6.x, 2.6.38
x86_64 for example.


Thanks in advance and regards,
Adrián

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Setreuid distinction about (uid_t)-1
  2012-07-17  9:27 Setreuid distinction about (uid_t)-1 Adrián
@ 2012-07-17 13:52 ` Athanasius
  2012-07-17 14:13   ` Adrián
  0 siblings, 1 reply; 5+ messages in thread
From: Athanasius @ 2012-07-17 13:52 UTC (permalink / raw)
  To: Adrián, linux-kernel

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

On Tue, Jul 17, 2012 at 10:27:55AM +0100, Adrián wrote:
>         uid = atoi(argv[1]);
>         printf("%u\n",uid);
>         if (setreuid(uid,uid)==-1){
>                 printf("Setreuid to %u failed\n ",uid);
>                 perror("E");
>                 exit(1);
>         }
>         execve("/bin/sh",args,NULL);
> 
> I've been calling this binary with a bunch of different uid numbers,
> and I came across this weird behaviour with the (uid_t) -1 value:

  From the man page:

       Supplying a value of -1 for either the real or effective user ID forces
       the system to leave that ID unchanged.
	
> adrian@home-pc:~$ /tmp/suid-tests
> Usage: /tmp/suid-tests target_uid
> adrian@home-pc:~$ /tmp/suid-tests 0
> 0
> Setreuid to 0 failed
> E: Operation not permitted
> adrian@home-pc:~$ /tmp/suid-tests -1
> 4294967295

  So this succeeded, by actually doing nothing.

> $ id
> uid=1000(adrian) gid=1000(adrian)
> groups=1000(adrian),4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin)
> adrian@home-pc:~$ /tmp/suid-tests -2
> 4294967294
> Setreuid to 4294967294 failed
> E: Operation not permitted
> adrian@home-pc:~$ /tmp/suid-tests -3
> 4294967293
> Setreuid to 4294967293 failed
> E: Operation not permitted
> 
> If the binary is setuid, the -1 call effectively rises the euid to
> root (0), although other arbitrary values are properly being set:

  Because, again, -1 asks to leave things as is.  And as you've made
the binary setuid and owned by root when you run it euid is set to 0, and
the -1 leaves it alone.

> adrian@home-pc:~$ ls -hl /tmp/suid-tests
> -rwsr-x--- 1 root adrian 8,5K 2012-07-17 10:53 /tmp/suid-tests
> adrian@home-pc:~$ /tmp/suid-tests -1
> 4294967295
> # id
> uid=1000(adrian) gid=1000(adrian) euid=0(root)
> groups=0(root),4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin),1000(adrian)

  Yup, totally as expected.

> adrian@home-pc:~$ /tmp/suid-tests -2
> 4294967294
> $ id
> uid=4294967294 gid=1000(adrian)
> groups=4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin),1000(adrian)

  -2 isn't a magic value, but as you're euid == 0 the kernel will do
what you asked and set uid to '-2', with some signed/unsigned conversion
going on you get 4294967294.

-- 
- Athanasius = Athanasius(at)miggy.org / http://www.miggy.org/
                  Finger athan(at)fysh.org for PGP key
	   "And it's me who is my enemy. Me who beats me up.
Me who makes the monsters. Me who strips my confidence." Paula Cole - ME

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Setreuid distinction about (uid_t)-1
  2012-07-17 13:52 ` Athanasius
@ 2012-07-17 14:13   ` Adrián
  2012-07-17 16:24     ` Theodore Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: Adrián @ 2012-07-17 14:13 UTC (permalink / raw)
  To: Adrián, linux-kernel

On Tue, Jul 17, 2012 at 2:52 PM, Athanasius <link@miggy.org> wrote:
> On Tue, Jul 17, 2012 at 10:27:55AM +0100, Adrián wrote:
>>         uid = atoi(argv[1]);
>>         printf("%u\n",uid);
>>         if (setreuid(uid,uid)==-1){
>>                 printf("Setreuid to %u failed\n ",uid);
>>                 perror("E");
>>                 exit(1);
>>         }
>>         execve("/bin/sh",args,NULL);
>>
>> I've been calling this binary with a bunch of different uid numbers,
>> and I came across this weird behaviour with the (uid_t) -1 value:
>
>   From the man page:
>
>        Supplying a value of -1 for either the real or effective user ID forces
>        the system to leave that ID unchanged.
>
>> adrian@home-pc:~$ /tmp/suid-tests
>> Usage: /tmp/suid-tests target_uid
>> adrian@home-pc:~$ /tmp/suid-tests 0
>> 0
>> Setreuid to 0 failed
>> E: Operation not permitted
>> adrian@home-pc:~$ /tmp/suid-tests -1
>> 4294967295
>
>   So this succeeded, by actually doing nothing.
>
>> $ id
>> uid=1000(adrian) gid=1000(adrian)
>> groups=1000(adrian),4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin)
>> adrian@home-pc:~$ /tmp/suid-tests -2
>> 4294967294
>> Setreuid to 4294967294 failed
>> E: Operation not permitted
>> adrian@home-pc:~$ /tmp/suid-tests -3
>> 4294967293
>> Setreuid to 4294967293 failed
>> E: Operation not permitted
>>
>> If the binary is setuid, the -1 call effectively rises the euid to
>> root (0), although other arbitrary values are properly being set:
>
>   Because, again, -1 asks to leave things as is.  And as you've made
> the binary setuid and owned by root when you run it euid is set to 0, and
> the -1 leaves it alone.
>
>> adrian@home-pc:~$ ls -hl /tmp/suid-tests
>> -rwsr-x--- 1 root adrian 8,5K 2012-07-17 10:53 /tmp/suid-tests
>> adrian@home-pc:~$ /tmp/suid-tests -1
>> 4294967295
>> # id
>> uid=1000(adrian) gid=1000(adrian) euid=0(root)
>> groups=0(root),4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin),1000(adrian)
>
>   Yup, totally as expected.
>
>> adrian@home-pc:~$ /tmp/suid-tests -2
>> 4294967294
>> $ id
>> uid=4294967294 gid=1000(adrian)
>> groups=4(adm),20(dialout),24(cdrom),46(plugdev),109(lpadmin),110(sambashare),111(admin),1000(adrian)
>
>   -2 isn't a magic value, but as you're euid == 0 the kernel will do
> what you asked and set uid to '-2', with some signed/unsigned conversion
> going on you get 4294967294.
>
> --
> - Athanasius = Athanasius(at)miggy.org / http://www.miggy.org/
>                   Finger athan(at)fysh.org for PGP key
>            "And it's me who is my enemy. Me who beats me up.
> Me who makes the monsters. Me who strips my confidence." Paula Cole - ME
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAlAFbgAACgkQSEDmQuIYzh2dSACfT+xjClQw/I68T7PnaF1W32B6
> kNcAmQHtVTb0S+oc2TIwy0uVZAO6K2Vc
> =7GG0
> -----END PGP SIGNATURE-----
>

Thanks a lot Athanasius. What I still can't see is why is the -1
exception there, as I assume that if you want to leave one of the ids
unchaged you can call:

setreuid(0,geteuid());

If you want to leave euid unchanged, right? Is there a need or reason
to be doing this differentiation in the setreuid code?

Thanks again,
Adrián

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Setreuid distinction about (uid_t)-1
  2012-07-17 14:13   ` Adrián
@ 2012-07-17 16:24     ` Theodore Ts'o
  2012-07-17 16:56       ` Adrián
  0 siblings, 1 reply; 5+ messages in thread
From: Theodore Ts'o @ 2012-07-17 16:24 UTC (permalink / raw)
  To: Adrián; +Cc: linux-kernel

On Tue, Jul 17, 2012 at 03:13:18PM +0100, Adrián wrote:
> 
> Thanks a lot Athanasius. What I still can't see is why is the -1
> exception there, as I assume that if you want to leave one of the ids
> unchaged you can call:
> 
> setreuid(0,geteuid());
> 
> If you want to leave euid unchanged, right? Is there a need or reason
> to be doing this differentiation in the setreuid code?

Unix systems for multiple decades have done things this way, and it's
ensrined in POSIX and the Single Unix Specification.  Changing it
would potentially open up security holes for programs which expect the
standard-specificed behavior.

(Note, BTW, that decades ago system calls weren't cheap, and CPU's
were much slower, and that may have driven the historical behavior.
Sometimes we get forget how spoiled we are that Linux's system call
overhead is as low as it is...)

					- Ted

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Setreuid distinction about (uid_t)-1
  2012-07-17 16:24     ` Theodore Ts'o
@ 2012-07-17 16:56       ` Adrián
  0 siblings, 0 replies; 5+ messages in thread
From: Adrián @ 2012-07-17 16:56 UTC (permalink / raw)
  To: Theodore Ts'o, Adrián, linux-kernel

On Tue, Jul 17, 2012 at 5:24 PM, Theodore Ts'o <tytso@mit.edu> wrote:
> On Tue, Jul 17, 2012 at 03:13:18PM +0100, Adrián wrote:
>>
>> Thanks a lot Athanasius. What I still can't see is why is the -1
>> exception there, as I assume that if you want to leave one of the ids
>> unchaged you can call:
>>
>> setreuid(0,geteuid());
>>
>> If you want to leave euid unchanged, right? Is there a need or reason
>> to be doing this differentiation in the setreuid code?
>
> Unix systems for multiple decades have done things this way, and it's
> ensrined in POSIX and the Single Unix Specification.  Changing it
> would potentially open up security holes for programs which expect the
> standard-specificed behavior.
>
> (Note, BTW, that decades ago system calls weren't cheap, and CPU's
> were much slower, and that may have driven the historical behavior.
> Sometimes we get forget how spoiled we are that Linux's system call
> overhead is as low as it is...)
>
>                                         - Ted
Thanks for that Theo!
Adrian

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-07-17 16:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-17  9:27 Setreuid distinction about (uid_t)-1 Adrián
2012-07-17 13:52 ` Athanasius
2012-07-17 14:13   ` Adrián
2012-07-17 16:24     ` Theodore Ts'o
2012-07-17 16:56       ` Adrián

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox