This is the mail archive of the cygwin-developers@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] |
Continuing plugging security holes in the core of Cygwin... The PROCESS_DUP_HANDLE privilege is currently used in 3 different fashions in Cygwin: 1) It is necessary to connect to a master TTY. Processes creating master ttys currently give total access to themselves from everybody. 2) It is necessary to do reparenting. A child is given a parent handle that has PROCESS_DUP_HANDLE, and it dups the grandchild handle into the grandparent. 3) It is needed to send a signal to a process (and for various interprocess communication with pipes). AFAICS such code takes no action to allow access to the process, thus I would expect: 4) A setuid'ed child of a process that has not created a master tty cannot signal its parent (e.g. for SIGCHLD). Having PROCESS_DUP_HANDLE to a process effectively grants full access, including the right to modify memory. Thus we must be extremely careful with it. See the remark at then bottom of <http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc/base/process_security_and_access_rights.asp> Specifically, the statements: a) tty::common_init: !SetKernelObjectSecurity (hMainProc, DACL_SECURITY_INFORMATION, get_null_sd ())) must be eliminated. b) proc_subproc: if (!DuplicateHandle (hMainProc, hMainProc, vchild->hProcess, &vchild->ppid_handle, 0, TRUE, DUPLICATE_SAME_ACCESS) should be changed to give no access rights to the duplicated handle. This will still allow to check if the parent is alive, but not to signal it nor to reparent. A) To solve 4), the child should not try to open the parent process and duplicate the sig pipe handle. It must be given by its parent a duplicate of the parent's sig pipe handle and use it in sig_send. In case of an exec, that copy must be duplicated and passed to the grandchild during spawn. We have to be careful because the grand child should not use that pipe to signal its (grand)parent before reparenting as occurred. B) To solve 2), the grandparent must obtain a handle to the grandchild (in the space of the child), and duplicate it himself. Fortunately, this can easily be done by reusing the sig pipe to the parent from the child, as per paragraph A, to give the parent a handle to the grandchild (in the child process). After passing the handle, the child must remain alive long enough to allow the parent to duplicate the handle. So we either need an ACK from the parent to the child, or more simply the parent could TerminateProcess the child once reparenting is complete (the reparenting code would be moved near the very end of the child). C) One way to solve 1) is as follows. In practice processes want to open /dev/tty, which gets remapped to some /dev/ttyN, depending on the ctty. Opening /dev/ttyN can fail for security reasons, but in practice we can arrange for processes to have inherited handles to their ctty, that are created just after the master tty has been opened, before seteuid. So as in A), no duplication from the master pid is necessary to open /dev/tty. I am attaching some "proof of principle" code that does C). It works but I am not a tty expert and I expect bugs. I tested it (strace) with rxvt and telnetd. It also works with sshd, but sshd crashes under strace and gdb on my WinME so I couldn't check all the details. I won't proceed with more changes in this area until I hear from Chris. Pierre P.S. Is this normal? The 34064 appears to be garbage in the child. 161 5543587 [main] in.telnetd 265749139 fhandler_tty_slave::dup: incremented open_fhs 3 168 5543755 [main] in.telnetd 265749139 tty_list::connect_tty: ttynum (34064) out of range P.S. for Corinna: At 04:39 PM 9/26/2003 +0200, you wrote: >On Fri, Sep 26, 2003 at 09:41:17AM -0400, Pierre A. Humblet wrote: >> Corinna Vinschen wrote: >> > Somehow I'm missing a description why that's necessary and the >> > implications. >> > >> I am getting paranoid. Most often we duplicate DUPLICATE_SAME_ACCESS >> without thinking about what access is really needed. It would be a good >> discipline to ask ourselves what is needed and give just that. Here nothing >> is needed at all. > >This handle is only used to have a handle. It's never accessed, just >kept so that we not get the same Winpid again, right? If so, the >patch is ok, of course. I was just missing a description. Now I can explain more fully: this inheritable handle was giving full access to the parent, even though its use appears to be completely benign.
Attachment:
tty.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |