Tcl Source Code

View Ticket
Login
Ticket UUID: 2964715
Title: Problems with Safe Base exposed glob command
Type: Bug Version: obsolete: 8.5.8
Submitter: kjnash Created on: 2010-03-06 13:29:05
Subsystem: 33. Safe Base Assigned To: dkf
Priority: 6 Severity: Minor
Status: Closed Last Modified: 2020-07-21 00:45:30
Resolution: Fixed Closed By: kjnash
    Closed on: 2020-07-21 00:45:30
Description:
Version core-8-5-branch v 1.16.4.6
Using ActiveTcl 8.5.8.1

The command ::safe::AliasGlob has several problems.
- [glob *] reveals the contents of [pwd].
- The call from ::tcl::tm::UnknownHandler (to search ::tcl::tm module directories) fails.
- Its test for calls from tclPkgUnknown is ineffective.
- Its error messages are unhelpful.

A patch to the core-8-5-branch v 1.16.4.6 is attached.

# Description of alterations to ::safe::AliasGlob from core-8-5-branch v 1.16.4.6
#
# (1) - add a comment header
# (2) - change test for "pkgIndex.tcl" and move it so that it is effective
# (3) - give a meaningful error message if pkgIndex.tcl is detected
# (4) - give a meaningful error message if -directory is used twice
# (5) - supply a value for dir in cmd
# (6) - require option -directory
#       - This is a change to the (undocumented) behaviour of the exposed glob
#         command.
#       - The change is compatible with the required use cases for tm.tcl and
#         (if the pkgIndex.tcl test is removed in future) package.tcl.
#       - It is a good fit to the Safe Base idea that operations are permitted
#         only in certain directories.

Closed Bug 2906841 is related.
User Comments: kjnash added on 2020-07-21 00:45:30:
Most of these fixes were applied.  Any issues mentioned here but unresolved are discussed in bug reports submitted in July 2020.

kjnash added on 2012-06-04 02:50:28:
1. Bug in AliasGlob: it does "-join" itself, and so it does not need to pass "-join" to the glob command

   Further bugs in safe.tcl -

2. Spurious directories in module path (test failure safe-14.1)
3. Does not clean up correctly after deleting a slave that has Safe Base sub-interpreters
4. init.tcl inappropriately adds un-tokenized directories to a safe slave's auto_path

A patch that fixes these bugs, and provides suitable tests, is provided on feature request 2964719.

johannes-kuhn added on 2012-05-17 23:30:26:
An alternative to provide a save subset of glob is to search for tm packages in the parent.

This approach would solve the huge glob mess, and it allows that the safe base uses it's original security policy, so it would be not longer necessary to allow the access to files in subdirs of the tokens (like tm does when requesting [package require tcl::chan::fifo])

I know recreating the tm for the safe base can be much work, but it might be worth the work.

dkf added on 2012-05-17 21:48:41:
OK, I think that all fixes it. The tests were *particularly* valuable. Left at Pending in case I've missed something.

(Globbing is complicated, and the amount of it we want to allow in safe interpreters is strictly limited — just enough to allow finding packages, no more. This makes the aliased-glob we use a very tricky piece of Tcl code indeed.)

johannes-kuhn added on 2010-05-20 00:12:11:
Glob on safe base currently broken, see bug #3004191.
With the patch from #3004191 glob * still returns the contents of [pwd]

Greetings.

kjnash added on 2010-03-10 23:42:54:
More bugs (B) in ::safe::InterpSetConfig - when the safe interpreter is given the same module path as the master, the path order is reversed, and subdirectories are added not only to the access path but also unnecessarily to the module path.

safe.tcl.patch2 is a patch against {core-8-5-branch v 1.16.4.6 patched by safe.tcl.patch} that fixes bugs (B).

More bugs (C) in ::safe::AliasGlob -
(C1) glob's option -nocomplain protects against error return if there are no matches; it should not protect against errors in arguments, e.g. no pattern
(C2) -join (if present) need not be the last non-pattern argument
(C3) AliasGlob tests that the directory part of a (compounded) glob pattern belongs to the access path; the test fails if the directory has a glob wildcard.

safe.tcl.patch3 is a patch against {core-8-5-branch v 1.16.4.6 patched by safe.tcl.patch, safe.tcl.patch2} that fixes bugs (C).

safe.tcl.patch1-3 is a cumulative patch against core-8-5-branch v 1.16.4.6 that fixes all bugs mentioned so far in 2964715.

safe-extras.test has tests intended for safe.test.  Tests 12.* are modified from safe.test for the patches provided here (notably altered error messages).  Tests 13.*, 14.* deal with bugs (C) and (B) respectively.

kjnash added on 2010-03-10 23:39:22:

File Added - 366150: safe-extras.test

kjnash added on 2010-03-10 23:38:58:

File Added - 366149: safe.tcl.patch1-3

kjnash added on 2010-03-10 23:38:38:

File Added - 366148: safe.tcl.patch3

kjnash added on 2010-03-10 23:38:16:

File Added - 366147: safe.tcl.patch2

johannes-kuhn added on 2010-03-10 01:15:18:
See also http://sourceforge.net/tracker/?func=detail&aid=2906841&group_id=10894&atid=110894

kjnash added on 2010-03-06 22:49:08:
I've built 8.6 from CVS, and it has similar problems, plus some more of its own.

It has replaced 8.5's

    if {[catch {
::interp invokehidden $slave glob {*}$cmd
    } msg]} {
Log $slave $msg
return -code error "script error"
    }

with

    try {
::interp invokehidden $slave glob {*}$cmd
    } on error msg {
Log $slave $msg
return -code error "script error"
    }

and so the result of the glob is thrown away if there is no error.

Why not stick with the [catch] formulation?  As well as being familiar, it will not be necessary (yet) to maintain separate versions of safe.tcl for 8.5 and 8.6.

dkf added on 2010-03-06 21:10:22:
I've been rewriting quite a bit of AliasGlob for 8.6, so I'll take over control of it and have a proper look. (If you can, testing whether the problems you've observed with 8.5.8 also occur with the CVS HEAD - a recent 8.6b1.1 will do - then that would make my life easier. Thanks, and I understand if that's not practical.)

kjnash added on 2010-03-06 20:29:06:

File Added - 365637: safe.tcl.patch

Attachments: