This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fixing a security hole in pinfo.


Christopher Faylor wrote:
> 
> On Thu, Sep 11, 2003 at 12:27:00AM -0400, Pierre A. Humblet wrote:
> >At 12:15 AM 9/11/2003 -0400, you wrote:
> >>On Thu, Sep 11, 2003 at 12:05:42AM -0400, Pierre A. Humblet wrote:
> >>>The flag PID_MAP_RW is added in the few pinfo constructors
> >>>that need to be write into _pinfo if it exists.
> >>>[snip]
> >>>diff -u -p -r1.166 exceptions.cc
> >>>--- exceptions.cc    10 Sep 2003 17:26:12 -0000      1.166
> >>>+++ exceptions.cc    11 Sep 2003 03:40:57 -0000
> >>>@@ -610,7 +610,7 @@ sig_handle_tty_stop (int sig)
> >>>      its list of subprocesses.  */
> >>>   if (my_parent_is_alive ())
> >>>     {
> >>>-      pinfo parent (myself->ppid);
> >>>+      pinfo parent (myself->ppid, PID_MAP_RW);
> >>>       if (NOTSTATE (parent, PID_NOCLDSTOP))
> >>>     sig_send (parent, SIGCHLD);
> >>>     }
> >>
> >>The above won't need to be RW when I check in my new signal changes.
> >>(Not that there won't be other inheritance type problems)
> >
> >Yep, I kind of suspected that, but it's still needed now.
> >I count on your solution to solve the issue of seteuid'ed children.
> >In fact, does your solution ever write to a remote _pinfo?
> >The PID_MAP_RW flag may have a very short life!
> 
> The parent still needs to write status flags (as in proc_subproc) in its
> children processes doesn't it?  Is that an issue here?  Otherwise,
> you're right.  I think that only parent processes will need to open this
> read/write.  I suspect there's still a security hole there, somewhere.

It's OK, I think. pchildren[nchildren] was created by the parent 
and created _pinfo's are always RW. In a patch to come the security 
attributes will give access to both parent and child in the seteuid case.
I don't see a security hole there, once all patches are in.

I take back my statement about PID_MAP_RW having a short life, it remains
necessary in setpgid. However in that case (pid is itself or a child) the 
current process already has an open _pinfo for the pid in question,
thus it's not really necessary to open a copy. So PID_MAP_RW will have
a short life only if we modify setpgid to use the existing _pinfo.

Pierre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]