MIME-Version: 1.0
From: Andy Lutomirski
Date: Wed, 24 Jun 2015 12:48:40 -0700
Subject: Re: Group man to group root privilege escalation
To: Kees Cook
Cc: halfdog, "security at kernel.org", Al Viro
Content-Type: text/plain; charset=UTF-8

On Wed, Jun 24, 2015 at 11:04 AM, Kees Cook wrote:
> On Tue, Jun 23, 2015 at 11:08 PM, halfdog wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Dear Sir or Madam,
>>
>> During testing following issue was detected: on Ubuntu Vivid with given
>> kernel the user "man/man" may escalate privileges to "man/root". See [1]
>>
>> The kernel under test was:
>>
>> $ cat /proc/version
>> Linux version 3.19.0-18-generic (buildd@lamiak) (gcc version 4.9.2
>> (Ubuntu 4.9.2-10ubuntu13) ) #18-Ubuntu SMP Tue May 19 18:30:59 UTC 2015
>>
>> There might be multiple bugs involved in this escalation and I'm not
>> sure if the same technique might be applicable to other targets. Also
>> the root cause might be wrong fs-permissions and not the kernel itself.
>> Hence I'm not sure how to rate this issue and deal with it.
>>
>> Could you please give it a short look and inform me if I should kept
>> this embargoed until fixed or if you rate this low impact and public
>> discussion and fixing might be more appropriate.
>>
>> Kind Regards,
>> hd
>>
>> [1]
>> http://www.halfdog.net/Security/2015/SetgidDirectoryPrivilegeEscalation/
>
> Thanks for sending this our way!
>
> This is an interesting combination of weird behaviors. The behaviors I see are:
>
> 1) open(..., O_RDWR | O_CREAT, 07777) doesn't strip set*id flags
>
>   int fd = open("testing", O_RDWR|O_CREAT, 07777);
>
> $ id
> uid=6(man) gid=12(man) groups=12(man)
> $ ls -ld . testing
> drwxr-sr-x 101 man root 4096 Jun 24 10:38 ./
> -rwsrwsr-t   1 man  root       0 Jun 24 10:38 testing*

I don't know whether this is a bug, but it sure seems weird in a
setgid directory.

>
> 2) seek past end doesn't strip set*id flags (since nothing has called
> write or truncate, target is still 0 bytes).
>
>   lseek(fd, 100, SEEK_SET);

Sounds like a bug to me.

> 3) glibc's ld-linux loader reports argv[0] on errors:
>
>       struct rlimit limits;
>
>       limits.rlim_cur = 1<<22;
>       limits.rlim_max = limits.rlim_cur;
>       setrlimit(RLIMIT_AS, &limits);
>
>       argv[0] = "blah";
>       argv[1] = "NULL";
>       execve("/bin/mount", argv, NULL);
>
> blah: error while loading shared libraries: libselinux.so.1: failed to
> map segment from shared object: Cannot allocate memory

So what?  There will never be a shortage of vectors for this.  My
personal favorite is crontab -l.

>
> 4) the writes originating from a root process don't strip set*id flags:
>
>   struct rlimit limits;
>   int fd = open("testing", O_RDWR|O_CREAT, 07777);
>
>   limits.rlim_cur = 1<<22;
>   limits.rlim_max = limits.rlim_cur;
>   setrlimit(RLIMIT_AS, &limits);
>
>   dup2(fd, 1);
>   dup2(fd, 2);
>   argv[0] = "blah";
>   argv[1] = "NULL";
>   execve("/bin/mount", argv, NULL);

If I had a penny for every time a write(2) implementation checked
current's credentials but somehow avoided causing a security problem
as a result, I'd have no money at all.

I seriously want to write something to make this trigger a WARN.

>
> $ ls -l testing
> -rwsrwsr-t 1 man root 126 Jun 24 10:53 testing*
> $ cat testing
> blah: error while loading shared libraries: libselinux.so.1: failed to
> map segment from shared object: Cannot allocate memory
>
> So, by using RLIMIT_FSIZE to control the length of writes (via
> ld-linux's error message from RLIMIT_AS), argv[0] is written to
> arbitrary locations  (due to seek location) in the file, all without
> clearing the set*id bits.
>
> It seems to me that the solutions could be either:
>
> a) fix open() to strip the set*id flags at creation time
>
> b) fix the test in write() for clearing set*id flags to look at who
> originally opened the file, not who is writing to it
>
> I suspect "b" may be totally wrong, but I'm still thinking about it.
> And it feels like "a" is missing something I haven't thought of that
> blocks that from being a solution.

I think we should do "b" at a bare minimum.  We have f_cred for a reason.

--Andy