Tk Source Code

This bug is difficult to assign to a category because it requires modifications to the code for four different widgets, and not to the Safe Base itself.

The purpose of a Safe Base interpreter is to run untrusted code.  Therefore, in a Safe Base interpreter with Tk, the widgets must not be able to write to the PRIMARY selection.  This capability would allow untrusted code to poison the PRIMARY selection with arbitrary values that might be used by the master interpreter, or by other processes.

The widgets that are permitted to write to the PRIMARY selection (text, entry, listbox, ttk::entry) examine the option -exportselection to determine whether they are allowed to do so.  When they perform this test, they must also check whether they are running in a safe interpreter, and if so, they must not write to the PRIMARY selection.

Tk 8.6.6 widgets do not perform this check.

The attached file primary-selection-problem.tcl shows how a Safe Base interpreter can poison the PRIMARY selection.

The attached file primary-selection-demo.tcl is useful for testing whether a Safe Base interpreter can write to the PRIMARY selection.

Branch [http://core.tcl.tk/tk/timeline?r=bug-de156e9efe|bug-de156e9efe] provides a fix for the bug.

Propagated to revised_text. All new tests do pass in the revised_text branch as well.

Thanks for this new revision of the tests file. i have now checked that it runs 100% OK on both Windows 7 and macOS.

I have merged the bigfix branch into core-8-6-branch and trunk.

Many thanks for this hard work Keith, that was something!

Working test file committed as [043985b1950b9].

The problem was that when Tk is tested without installation, the usual auto_path loading mechanism cannot load Tk because the tk library binary is not where its pkgIndex.tcl says it is.  I found the workaround code (which uses ::load) in another test file, and copied that with modifications.

I haven't tested on macOS, but the new test file works with Linux and Windows.

If Tk has been installed, then "make test" in the build directory will detect the installation under Linux, but not under Windows, which may be the reason for the difference that you found.

This is a weird one!  I tested on Windows 7 (as well as Linux), and all the tests passed.  But I sourced the test file into wish.exe, and this technique works for you too!

With nmake, the error message "can't find package Tk" indicates that the unsafe interpreter was created without the correct "lib" directory in its $auto_path.

I suspect that there is a bug in the Makefile for "nmake test".  I use MinGW/MSYS, and this has a similar problem.

I will continue investigating.

How strange! On Windows, in the bugfix branch running the test file from tclsh86tg.exe:

% source tests/safePrimarySelection.test
safePrimarySelection.test:      Total   60      Passed  60      Skipped 0       Failed  0

But running it through:

nmake -f makefile.vc test OPTS=symbols TCLDIR=%MYTCL% TESTFLAGS="-file safePrimarySelection.test -match safePrimarySelection-2.1"

==== safePrimarySelection-2.1 unsafe slave interpreter, text, no existing select
ion FAILED
==== Contents of test case:

    set int2 slave2
    interp create $int2
    $int2 eval $::_test_tmp::script
    $int2 eval ::_test_tmp::tryText
    $int2 eval ::_test_tmp::getPrimarySelection

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: can't find package Tk
    while executing
"package require Tk"
    invoked from within
"$int2 eval $::_test_tmp::script"
    ("uplevel" body line 4)
    invoked from within
"uplevel 1 $script"
---- errorCode: TCL PACKAGE UNFOUND
==== safePrimarySelection-2.1 FAILED

I really have no idea about what's going on here. Help appreciated, thanks!

I have read your added test file I think it is really complete. Results:

1. Running the new tests on Linux Debian 8: all tests pass.

2. Running the new tests on Windows Vista: 20 tests fail (safePrimarySelection-2.1 to -2.10 and safePrimarySelection-5.1 to -5.10), spitting:

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: can't find package Tk
    while executing
"package require Tk"
    invoked from within
"$int2 eval $::_test_tmp::script"
    ("uplevel" body line 4)
    invoked from within
"uplevel 1 $script"
---- errorCode: TCL PACKAGE UNFOUND

Problem on Windows appears to be "package require Tk" at the beginning of ::_test_tmp::script when run in an unsafe interpreter. It does not find Tk . How strange. It works from the master interp and from a safe interp (since you incantated safe::loadTk in the latter).

3. Running the new tests on macOS segfaults because it picks the wrong (i.e. system-installed) version of Tk in safePrimarySelection-2.1:

safePrimarySelection.test
objc[95073]: Class TKApplication is implemented in both /Users/fvogel/Documents/tcltk/fossil/build/tk/Development/Tk.framework/Versions/8.6/Tk (0x107045d38) and /System/Library/Frameworks/Tk.framework/Versions/8.5/Tk (0x10d3081f8). One of the two will be used. Which one is undefined.
objc[95073]: Class TKMenu is implemented in both /Users/fvogel/Documents/tcltk/fossil/build/tk/Development/Tk.framework/Versions/8.6/Tk (0x107045dd8) and /System/Library/Frameworks/Tk.framework/Versions/8.5/Tk (0x10d308248). One of the two will be used. Which one is undefined.
objc[95073]: Class TKContentView is implemented in both /Users/fvogel/Documents/tcltk/fossil/build/tk/Development/Tk.framework/Versions/8.6/Tk (0x107045d88) and /System/Library/Frameworks/Tk.framework/Versions/8.5/Tk (0x10d308298). One of the two will be used. Which one is undefined.
objc[95073]: Class TKWindow is implemented in both /Users/fvogel/Documents/tcltk/fossil/build/tk/Development/Tk.framework/Versions/8.6/Tk (0x107045e28) and /System/Library/Frameworks/Tk.framework/Versions/8.5/Tk (0x10d3082e8). One of the two will be used. Which one is undefined.
objc[95073]: Invalid or prematurely-freed autorelease pool 0x7fd6a9041068.
/bin/sh: line 1: 95073 Abort trap: 6           ./tktest /Users/fvogel/Documents/tcltk/fossil/tk/unix/../tests/all.tcl -geometry +0+0 -file safePrimarySelection.test
make[2]: *** [test-classic] Error 134
make[1]: *** [test-tk] Error 2
make: *** [test-develop] Error 2

Probably the same root cause on macOS than on Windows. Wondering why it works on Linux though.

I'll have a look but if you have ideas in the meantime, sharing those will be appreciated.

Wow this is a full lot of new tests, many thanks!

I will need a few days to review all this, please stay tuned.

Tests now supplied as file tests/safePrimarySelection.test in branch [bug-de156e9efe].

Will do.

Keith, do you think you can come up with new testcases to check that the PRIMARY selection cannot be changed from a safe interp?

That's the only thing missing before we can merge I think.

Note for myself and/or Gregor: this fix should be propagated to revised_text.

primary-selection-demo2.tcl errors out for me because the Thread package is not available. But no problem, I have understood.

primary-selection-problem2.tcl errors out for me for a more obscure reason I don't have the time to analyze right now:

  invalid command name "::safe::loadTk"

The documentation text you propose look very good to me, I believe it can go as is in selection.n, just before the "EXAMPLES" section.

I believ new tests should be added (in safe.test), to check that the PRIMARY selection cannot be changed from a safe interp.

Re: fvogel 2017-06-21 21:08:44:

I have revised the demo scripts to demonstrate the issues more clearly, including on Windows, which I had previously ignored because its PRIMARY selection, not being system-wide, has limited use.

primary-selection-demo2.tcl opens windows for four different interpreters: the master, an unsafe slave, a safe slave, and the master interpreter of a second thread.  The purpose of this demo is to allow experimentation.  It helped me to understand the limitations of the PRIMARY selection on the Windows platform.

primary-selection-problem2.tcl explains its purpose more clearly than version 1.  Every 5s an unmapped window will steal the PRIMARY selection and poison it with the string "PAYLOAD".  On Linux, this change is signalled by the deselection of any visible text, but the main point is that the PRIMARY selection is not what you think it is.

On Windows, the PRIMARY selection is shared only between the master interpreter and its slaves.  primary-selection-problem2.tcl launches another window so the user can test pasting the PRIMARY selection - because windows from other processes will not work.  Again, the main point is that after 5s the PRIMARY selection is poisoned.  On Windows, the change is NOT signalled by the deselection of text in the visible widgets, because Windows allows different widgets to have a selection at the same time.

Here is a first attempt at documentation.  Please can we try to agree the approximate text before attempting formatting for doc/selection.n?  I have written sections for "WIDGET FACILITIES" and "PORTABILITY ISSUES" mainly to aid my own understanding: please include these if you feel they are useful.  I have not yet tried MacOS/Aqua.


WIDGET FACILITIES

A text, entry, ttk::entry, listbox, spinbox or ttk::spinbox widget can have option -exportselection 1, meaning (in an unsafe interpreter) that a selection made in one of these widgets is automatically written to the PRIMARY selection.

A GUI event, for example <<PasteSelection>>, can copy the PRIMARY selection to certain widgets.  This copy is implemented by a widget binding to the event.  The binding script makes appropriate calls to the {selection} command.

PORTABILITY ISSUES

On X11, the PRIMARY selection is a system-wide feature of the X server, allowing communication between different processes that are X11 clients.

On Windows, the PRIMARY selection is not provided by the system, but only by Tk, and so it is shared only between windows of a master interpreter and its unsafe slave interpreters.  It is not shared between interpreters in different processes or different threads.  Each master interpreter has a separate PRIMARY selection that is shared only with its unsafe slaves.

SECURITY

An unsafe interpreter cannot read from the PRIMARY selection because its {selection} command is hidden.  For this reason the PRIMARY selection cannot be written to the Tk widgets of a safe interpreter.

Tk widgets can have option -exportselection 1, but in a safe interpreter this option has no effect: writing from the widget to the PRIMARY selection is disabled.

These are security features.  A safe interpreter may run untrusted code, and it is a security risk if this untrusted code can read or write the PRIMARY selection used by other interpreters.

Re fvogel 2017-06-30 20:50:49:

I will post revised versions of the demo scripts shortly.

While browsing the web, I used the PRIMARY selection to copy a URL from a plain-text document to the browser's address bar. I found I had pasted "PAYLOAD" and then I realised I had forgotten to close the running demo primary-selection-problem2.tcl.

A similar problem arises if pasting to a terminal: in that case "rm -rf *" is a danger.

So yes, this is the reason for proposing the change.  Without it, a malicious Tclet can interfere with activity in other X11 clients.

Aside of the missing documentation, I'm starting to wonder a bit why we should prevent safe interps from changing the X selection.

Looking around in the wiki, I could find this page mentioning that A malicious script could replace the contents of the clipboard with the string “rm -r *” and lead to surprises when the contents of the clipboard are pasted.

Is it the reason why you're proposing this change? Could you perhaps elaborate on which grounds this was grown? Any real world case into which you ran?

More comments welcome, from anyone seeing light here.

On Windows the demo script does not show any issue, but on Linux I see the selection disappear every 5 seconds. Select again manually, and it disappears again. I understand that this is what you wanted to demonstrate.

Having clarified this, I'm fine with preventing safe interpreters from fiddling with the PRIMARY X selection, but I believe that this should be documented, perhaps in the selection man page. What do you think?