Ticket UUID: | 502e74e9ad198e5239092356333aceba5c6ab47a | |||
Title: | crash for untrusted X connections (for ssh: ForwardX11Trusted no) | |||
Type: | Bug | Version: | 8.5 | |
Submitter: | anonymous | Created on: | 2014-01-05 07:30:44 | |
Subsystem: | 71. Error Handling | Assigned To: | fvogel | |
Priority: | 5 Medium | Severity: | Important | |
Status: | Closed | Last Modified: | 2018-02-11 12:04:57 | |
Resolution: | Fixed | Closed By: | fvogel | |
Closed on: | 2018-02-11 12:04:57 | |||
Description: |
Changing the InterpRegistry atom on the root window is not allowed for untrusted clients. One way to work around this might be to install xlib error handler which ignores these errors. This fixes the problem for me. In unix/tkUnixEvent.c *---------------------------------------------------------------------- * * XlibErrorHandler -- * * This function is called for non-fatal X errors. * * Results: * None. * * Side effects: * Calls default error handler for most errors. * *---------------------------------------------------------------------- */ static XErrorHandler OldXlibErrorHandler = NULL; static int XlibErrorHandler(Display *d, XErrorEvent *e) { /* Don't fail for BadAccess errors caused by XChangeProperty. * Changing the "InterpRegistry" atom on the root window fails * for untrusted connections. */ // X_ChangePropery = 18 if ((BadAccess == e->error_code) && (18 == e->request_code)) return 0; if (NULL != OldXlibErrorHandler) OldXlibErrorHandler(d, e); return 0; } And just install it before opening the display: TkDisplay * TkpOpenDisplay( CONST char *displayNameStr) { TkDisplay *dispPtr; OldXlibErrorHandler = XSetErrorHandler(XlibErrorHandler); Display *display = XOpenDisplay(displayNameStr); .... | |||
User Comments: |
fvogel added on 2018-02-11 12:04:57:
Merged to core-8-6-branch and trunk. And I confirm that it is now possible to 'make test' with an untrusted X connection without being stopped by X errors (but of course with more failures due to the untrusted connection). fvogel added on 2018-02-11 12:00:06: OK, I have tried it following your instructions. Here are my findings: 1. In current core-8-6-branch (i.e. without any patch supposed to fix this bug): Running wish I get: francois@TM:~/Documents/tcltk/fossil/tk/unix$ ssh -X localhost francois@localhost's password: The programs included with the Debian GNU/Linux system are free software; the exact distribution terms for each program are described in the individual files in /usr/share/doc/*/copyright. Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent permitted by applicable law. You have new mail. Last login: Sun Feb 11 12:45:16 2018 from localhost francois@TM:~$ /home/francois/Documents/tcltk/fossil/tcltk/bin/wish8.6 X Error of failed request: BadAccess (attempt to access private resource denied) Major opcode of failed request: 18 (X_ChangeProperty) Serial number of failed request: 15 Current serial number in output stream: 18 francois@TM:~/Documents/tcltk/fossil/tk$ 2. Now with the bugfix branch: Running wish I get: francois@TM:~/Documents/tcltk/fossil/tk/unix$ ssh -X localhost francois@localhost's password: The programs included with the Debian GNU/Linux system are free software; the exact distribution terms for each program are described in the individual files in /usr/share/doc/*/copyright. Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent permitted by applicable law. You have new mail. Last login: Sun Feb 11 12:45:39 2018 from localhost francois@TM:~$ /home/francois/Documents/tcltk/fossil/tcltk/bin/wish8.6 % puts "This works!" This works! % exit francois@TM:~$ Therefore we definitely have a fix here. Will merge. fvogel added on 2018-01-31 07:30:16: Revised patch committed in the bugfix branch, many thanks. I will try this ASAP. chw added on 2018-01-30 22:50:07: A simplified test setup is: 1. a Debian 9 machine 2. the /etc/ssh/ssh_config must have ForwardX11 yes ForwardX11Trusted no 3. tests run through a local ssh connection using "ssh -X localhost" The unixsend2etc.patch is a refinement of unixsend.patch and allows to perform a "make test" without being stopped by X errors (and of course with more failures due to the untrusted connection). chw added on 2018-01-30 05:12:00: Steps to reproduce: 1. two machines required, one Debian (version 9?) 2. the Debian machine is the X display and logs into the other machine using ssh -X ... 3. the /etc/ssh/ssh_config of the Debian machine must have these entries: ForwardX11 yes ForwardX11Trusted no 4. starting the remote wish without patch through the ssh login fails with BadAccess X error very early when the wish wants to register its app name for send(n). Most likely the X error happens on XChangeProperty() in RegClose() in tkUnixSend.c when a property of the root window is to be changed and this is not allowed due to the ssh client setup and this throws the BadAccess X error. I must admit that I still don't quite overview the problem, too. But the part of the patch with reordering the init of the pending struct before calling AppendPropCarefully() is an obvious error. fvogel added on 2018-01-29 21:39:24: OK, since I promised I had a look at this issue. Net result is that I don't understand at all what this is about. I'm really not the right guy to check fix proposals here. I have nevertheless committed Christians's patch in a bugfix branch bug-502e74e9ad for easy review. I don't understand the issue here, I don't know how to reproduce it, I don't know how to check that it is fixed. But I fully trust Christian's opinion. If he, who is much more proficient on this than I am, states this is fixed with his patch then I will take this as granted and will merge. In the meantime, other reviewers welcome. Thanks! chw added on 2018-01-29 19:46:10: Not a nice solution. Please try my proposal instead. It uses Tk_(Create|Delete)ErrorHandler() when required and fixes an ordering problem around the call of AppendPropCarefully(). The re-order is required for 1. -sync mode for the display connection, i.e. Xlib debugging 2. when my proposed Tk_DeleteErrorHandler() change is used otherwise the event loop after AppendPropCarefully() locks up the application carefully. Patch is against core-8-6-branch. dgp added on 2018-01-29 15:19:09: http://lists.alioth.debian.org/pipermail/pkg-tcltk-devel/2018-January/003489.html fvogel added on 2018-01-29 14:57:04: I wouldn't pretend being code.petent on these matters but I can have a look. Where is the discussion you are referring to? Is there a reproduction script? dgp added on 2018-01-29 13:05:37: A message from Debian says this bug is still a problem in Tk 8.6.6, and that the patch suggestion here is an effective fix. Could a Tk maintainer please have a look? |