Tk Source Code

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);

....

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).

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.

Revised patch committed in the bugfix branch, many thanks.
I will try this ASAP.

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).

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.

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!

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.

http://lists.alioth.debian.org/pipermail/pkg-tcltk-devel/2018-January/003489.html

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?

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?