#2901 Buffer overrun in [subst]

obsolete: 8.4.7
closed-fixed
9
2004-10-26
2004-09-28
No

This is not a problem in the HEAD.

Repeatedly do this command. The error result is always
strange and occasionally it crashes. (Seems random which.)
subst {[set x 1;}

GDB claims crash happened like this:
Program received signal SIGSEGV, Segmentation fault.
0x0807e5ab in Tcl_SubstObj (interp=0x8103390,
objPtr=0x810bd10, flags=7)
at ../generic/tclCmdMZ.c:2578
2578 switch (*p) {
The 'p' pointer seems to have overflowed well past the
end of the objPtr->bytes parameter:
(gdb) print p
$1 = 0x812c000 <Address 0x812c000 out of bounds>
(gdb) print objPtr->bytes
$2 = 0x8127788 "[set x 1;"

Looks like a buffer overrun to me, and [subst] is often
exposed to external sources. => CRITICAL BUG

Discussion

  • miguel sofer

    miguel sofer - 2004-09-29

    Logged In: YES
    user_id=148712

    The problem seems to be in Tcl_EvalEx: it fails when it
    evaluates a nested script of unknown length (numBytes==-1)
    that is missing the closing ']' AND ends in ';' (a space
    after that seems not to trigger the buffer overflow).

    In any case, the attached patch to core-8-4-branch seems to
    fix it. There might be a more elegant to fix this ...

    Note that Tcl_EvalEx has been rewritten in HEAD; I did not
    yet check if the problemmight be there too.

     
  • Donal K. Fellows

    • labels: 105659 --> 45. Parsing and Eval
    • assigned_to: dgp --> msofer
     
  • Donal K. Fellows

    Logged In: YES
    user_id=79902

    Syntax error is reported wrongly. :^(

    When I did the following after applying your patch:
    subst "\[subst {};"
    I got this error:
    can't read "f": no such variable

     
  • miguel sofer

    miguel sofer - 2004-09-29

    Logged In: YES
    user_id=148712

    updated patch - still missing tests

     
  • miguel sofer

    miguel sofer - 2004-09-29
     
  • Donal K. Fellows

    Logged In: YES
    user_id=79902

    Fix looks good. Suggested test (you might want to give it a
    different test number):
    test subst-12.1 {nasty case} {
    list [catch {subst "\[subst {};"} msg] $msg
    } {1 {missing close-bracket}}

     
  • miguel sofer

    miguel sofer - 2004-09-29

    Logged In: YES
    user_id=148712

    Fix and tests committed to core-8-4-branch.

    Still to do: commit the new tests to HEAD, check if HEAD has
    the flaw too.

     
  • miguel sofer

    miguel sofer - 2004-09-30

    Logged In: YES
    user_id=148712

    tests added to HEAD; code review not yet done

     
  • Don Porter

    Don Porter - 2004-10-26

    Logged In: YES
    user_id=80530

    added more tests (subst-12.3,4)
    to show the effect of syntax errors
    on side effects.

    Test subst-12.3 illustrates a change
    in behavior from 8.4 to 8.5.

     
  • Don Porter

    Don Porter - 2004-10-26
    • assigned_to: msofer --> davygrvy
    • status: open --> closed-fixed
     
  • Don Porter

    Don Porter - 2004-10-26

    Logged In: YES
    user_id=80530

    corrected HEAD to have same
    subst-12.3 results as the 8.4 branch.

    At this point [subst] does not crash
    and is consistent between 8.4 and 8.5..
    Anyone wanted other mods to [subst]
    should file a separate report.

     
  • Don Porter

    Don Porter - 2004-10-26
    • assigned_to: davygrvy --> dgp