[Patch]: Fixing the PROCESS_DUP_HANDLE security hole (part 1).
Pierre A. Humblet
Thu Dec 11 02:54:00 GMT 2003
At 12:08 AM 12/9/2003 -0500, Christopher Faylor wrote:
>On Mon, Dec 08, 2003 at 11:36:01PM -0500, Christopher Faylor wrote:
>>I would have but the information that the fhandler contains the
>>controlling tty is lost by the time dup is called. Hmm. I guess I
>>could just check the io_handle. I'll do that.
>I did this but, in testing, found that there are still other problems
>with setsid assuming that it can close the cygheap ctty when there may
>still be open fds which wouldn't like that to happen.
>I'll work on that tomorrow.
OK, it works, but I found a few odd things.
This happens with rxvt -d :0 -e sh
1) The master process opens /dev/ptmx and gets tty3 with fd = 4
159 809780 [main] rxvt 46309609 open: open (/dev/ptmx, 0x8002)
164 809944 [main] rxvt 46309609 normalize_posix_path: src /dev/ptmx
158 810102 [main] rxvt 46309609 normalize_posix_path: /dev/ptmx = normalize_posix_path (/dev/ptmx)
157 810259 [main] rxvt 46309609 mount_info::conv_to_win32_path: conv_to_win32_path (/dev/ptmx)
159 810418 [main] rxvt 46309609 mount_info::conv_to_win32_path: src_path /dev/ptmx, dst \dev\ptmx, flags 0x2, rc 0
180 810598 [main] rxvt 46309609 build_fh_pc: fh 0x616945B0
200 810798 [main] rxvt 46309609 tty_list::allocate_tty: tty3 allocated
282 811080 [main] rxvt 46309609 tty::make_pipes: tty3 from_slave 0xB8, to_slave 0xA8
The forked process closes fd 4 but prints out that it closes tty2, althout it
correctly closes the handles of tty3
162 1213090 [main] rxvt 50291421 close: close (4)
161 1213251 [main] rxvt 50291421 fhandler_tty_common::close: tty2 <0xB8,0xA8> closing
169 1213420 [main] rxvt 50291421 delete_handle: nuking handle 'input_mutex'
159 1213579 [main] rxvt 50291421 delete_handle: nuking handle 'output_mutex'
196 1213775 [main] rxvt 50291421 close: 0 = close (4)
2) sysinternals shows a lot more handles than expected with the new dup method.
The reason is that the master process opens a slave /dev/tty3 as fd 5
(this is not its ctty).
The forked rxvt setsids, opens another slave tty3, which sets the ctty, but
it immediately closes that tty3.
It then proceeds to dup2(5, 0), dup2(5, 1) and dup2(5, 2).
Because the io_handle of 5 doesn't match the io_handle of the ctty, your
dup code doesn't kick in for those dups.
Why can't we compare get_unit() with myself->ctty to determine if we can use
the dup shortcut? (i.e. I don't understand the first quoted paragraph above).
Also it would be safer to increment open_fhs only after the dup succeeds.
More information about the Cygwin-patches