DragonFlyBSD

Shouldn't we rather remove the file and recreate a new file (which then
will be owned by root)?

cheers
simon

Hello Simon,

In my opinion checking for ownership is better. We are avoiding other
possible(?) bugs e.g. allowing to read files you don't own but resides
on a directory you own. I also noticed that non-root users trying to
coredump on other non-root users pre-created dumps fail silently.

By the way as seen in my patch, we wouldn't want to hard code != 0
because DragonFly may implement a type enforcement system or
authorization framework.

Up to you guys. I might be missing something.

Cheers,
Ed

On Feb 16, 2008 4:03 AM, Simon 'corecode' Schubert
<corecode@fs.ei.tum.de> wrote:

Eduardo Tongson wrote:

su

Password:
syslog: Feb 16 09:40:56 su: user to root on /dev/ttyd0

1. ./coredumper
   Segmentation fault (core dumped)
   syslog: Feb 16 09:41:14 kernel: pid 728 (coredumper), uid 0: exited
   on signal 11 (core dumped)
2. md5 coredumper.core
   MD5 (coredumper.core) = 68e3e5fee874e688c795537721a6b511
3. ls la coredumper.core
   -rw------ 1 user user 1003520 Feb 16 09:41 coredumper.core #

I was not able to test the below patch. Trivial enough to fix if broken.

--- kern_sig.c 2008-02-14 13:41:12.000000000 0800
++ kern_sig-20080216.c 2008-02-16 01:15:01.000000000 +0800
@ -2066,6 +2066,12 @ coredump(struct lwp *lp, int sig)
goto out1;
}

+ /* Don't dump to files current user does not own */

Shouldn't we rather remove the file and recreate a new file (which then
will be owned by root)?

cheers
simon

:Hello Simon,
:
:In my opinion checking for ownership is better. We are avoiding other
:possible(?) bugs e.g. allowing to read files you don't own but resides
:on a directory you own. I also noticed that non-root users trying to
:coredump on other non-root users pre-created dumps fail silently.
:
:By the way as seen in my patch, we wouldn't want to hard code != 0
:because DragonFly may implement a type enforcement system or
:authorization framework.
:
:Up to you guys. I might be missing something.
:
:Cheers,
: Ed

Yes, I agree, we definitely do not want to remove the file.  The reason
    is that it is possible (and likely) that a service process that forks
    may coredump multiple times, possibly even in parallel.  Possibly
    hundreds of times in parallel.  This is one reason why the coredump
    code gets an advisory lock on the file.

My only question re: this patch is whether it will work properly
    with sysctl kern.sugid_coredump.

-Matt

Hello Matt,

p->p_ucred->cr_uid is equal to the user/group ID of the process right?
I think it will work.

Ed

Ok, I've committed the basic ownership check to the coredump code for
this release. I did verify that sgid dumps still work (when enabled
via the sysctl).

I have not committed the group/other permissions check, at least not
     yet.  It seems a little excessive considering the access needed to
     exploit such a hole is of far greater consequence then any desire to
     use it to exploit the core dump path would be.

-Matt