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: [Patch]: Fixing the PROCESS_DUP_HANDLE security hole (part 1).


Pierre A. Humblet wrote:
>At 05:40 PM 12/7/2003 -0500, Christopher Faylor wrote:
>>On Mon, Sep 29, 2003 at 09:55:25PM -0400, Pierre A. Humblet wrote:
>>>Here is a patch that allows to open master ttys without giving
>>>full access to the process, at least for access to the ctty. 
>>>
>>>It works by snooping the ctty pipe handles and duplicating them
>>>on the cygheap, for use by future opens in descendant processes.
>>>
>>>It passes all the tests I tried, but considering my lack of knowledge
>>>about ttys, everything is possible.
>>
>>I checked in a variation of this patch.  It restructures the way
>>controlling tty is handled, making it a little easier to deal with
>>/dev/tty at the fhandler level.  I suspect that eventually there will
>>be a fhandler_ctty class but, for now, this seems to work.
>>
>>I'm not 100% certain that I got the close-on-exec stuff right but, fwiw,
>>it seems to work with the combination of ssh/zsh which is usually a
>>pretty tough test for this kind of thing.  I did check to make sure that
>>access to a tty is now not allowed from a non-privileged account thanks
>>to the tty.cc change below.
>>
>>Thanks for the patch and sorry for the delay.
>>
>>cgf
>
>It's mostly fine (rxvt and notty) but starting the following from DOS
>creates a slew of warning from the handler protection code (below).
>However the shell is functional.

When I tried the same dll on an NT4 the errors had disappeared. But that's
a question of luck, i.e. not creating a new handle that matches one that
will be incorrectly deleted multiple times.

I have now traced the problem: when /dev/tty is duplicated, myself->set_ctty
is called again (with the same everything, except new handles), and it 
blindly copies everything over, including the handles.
>From there on the test in fhandler_tty_slave::close () doesn't work as 
intended.
Either myself->set_ctty should be smarter, or fhandler_tty_slave::dup
could see if it's about the ctty and simply copy it.

Pierre

 


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