Tcl Source Code

[subst] produces garbage when the input contains an
incomplete command substitution:

  set lb {[}
  set a [subst $lb]

First saw this in Tcl 8.4 alpha something (CVS
snapshot), but the bug seems to go back as far as Tcl
8.0.

Also:
subst {[set a 1};# missing "]"

sets $a to 1 and returns 1; this should probably be an
error instead.

Logged In: YES 
user_id=148712

Tests added to HEAD

Logged In: YES 
user_id=80530

See Tcl Bug 533364 for the new bug.

Logged In: YES 
user_id=80530

Looks like I misdiagnosed it.
Really a bug in the new
TclGetIntForIndex() routine.

Logged In: YES 
user_id=146884

COMMENT ADDED AT REQUEST OF Don Porter by Roy Terry
In article <[email protected]>, Roy Terry 
wrote:
> % time {lindex [split this,that ,] 0]} 1000
> Tcl_AppendStringsToObj called with shared object
> 
> abnormal program termination

Probably a problem with the fix for Tcl Bug 495207.  Please 
post
this example as a followup comment there.

Logged In: YES 
user_id=148712

Patch committed; lowering priority but not closing yet: I'll
try to find better tests for this.

File Added - 18428: subst.patch2

Logged In: YES 
user_id=148712

This subst.patch2 reproduces the 7.6 behaviour.

Note that the unpatched tcl passes the tests under tcltest,
although it does overrun the buffer at the command line. New
tests should be devised ...

Logged In: YES 
user_id=72656

Actually, let me change my mind, since 7.6 does actually 
throw an error "missing close-bracket".  I think we should 
try to get back to 7.6-like behavior.

Logged In: YES 
user_id=72656

I don't think an error should be thrown here ...  Then 
again, I'm not sure what change in behavior would be "most 
compatible", but preventing the buffer overrun is important.

Logged In: YES 
user_id=79902

Question: why isn't the missing close bracket causing Tcl_EvalEx() to
return TCL_ERROR? It seems to me that there is something deeply odd
going on here, and that it seems deeply suspicious that the lack of a
bracket gets treated as a normal return...

(Note that there is definitely a need for something to be done here to
improve on the current situation w.r.t. handling of error cases, and
possibly add some kind of pre-parsing to allow for faster [subst]ing the
second time a string is fed through, but that's something for another
time.)

Logged In: YES 
user_id=68433


>Should it instead return with an error *before* setting a?

That's what Tcl 7.6 does:
| % subst {[set a 1}
| missing close-bracket
| % set a
| can't read "a": no such variable

Also, subst {[set a 1] [ set b 2} sets a, then raises
an error before setting b (in 7.6).

File Added - 15048: subst.patch

Logged In: YES 
user_id=148712

The error seems to be in Tcl_SubstObj(): it *assumes* that
bracket terms are properly terminated, and misbehaves when
this assumption is not verified.
I am enclosing a patch that resolves this issue. I am not
sure if it behaves properly though, I have a question as to
what should happen in case of a misformatted bracket term.
Specifically: if we request
   subst {[set a 1} ;# missing "]" 
this patch causes a to be set to 1, and *then* returns an
error. Should it instead return with an error *before*
setting a?

Donal, I'm assigning back to you for comment on this issue.

Logged In: YES 
user_id=79902

Looks like memory corruption in the hairiest bit of the parser (i.e. when the TCL_BRACKET_TERM flag is set);
is it *really* going past the end of the string?!? In any case, this is outside my domain...

Upped the severity as [subst] is sometimes used on untrusted data (though it should not be a general problem 
because anyone not setting -nocommands or controlling what has brackets is asking for trouble anyway) and might 
have some impact on some important Tcl applications (like webservers...)