Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#4252 buffer overflow in AppendUnicodeToUnicode() -> SEGFAULT

obsolete: 8.5.5
closed-fixed
Don Porter
9
2009-04-07
2009-01-08
Matthias Kraft
No

Hi *,

initially I got a core dump on AIX 5.2 in FlushChannel() of tclIO.c using Tcl 8.4.19.

I tracked it down to the memcpy() call in AppendUnicodeToUnicode() in tclStringObj.c that is getting a wrong destination pointer in case the attemptckrealloc() call (doubling available space for StringPtr) failed. Once I understood how this all comes to happen, the following simple script provokes this crash even on Linux ...

proc bar {} {
set f [string repeat "\xDE\xAD\xBE\xEF" 1024] ;# for the crash in AppendUnicodeToUnicodeRep()
#set f [string repeat "DEADBEEF" 1024] ;# for a crash in AppendUtfToUtfRep() that I have not investigated further
set r ""
set i 0

while {1} {
puts -nonewline stdout "$i\r"
set fl [string length $f]
if {($fl != [string length $r]) || 1} {
append r [string range $f 0 $fl]
}
incr i $fl
}
}
bar

I don't quite understand why it gets the wrong address there, yet.

Note: It may happen that the crash occurs at different positions, or leaves unreadable core dumps behind. This could be avoided with setting the environment variable MALLOC_CHECK_=2 on a fairly recent Linux system, or MALLOCTYPE=debug on AIX. The crash also depends on how much free memory is available.

If you need more information, core dumps, stack traces ... just ask.

One furher thing, I polluted the sources with printf()s, the crashing call to AppendUnicodeToUnicodeRep() generated:

+++ alloced tmpString (0x2ab8abe8) of size uallocated (67126290)
+++ copying 8192 bytes from 0x20b59ff8 to 0x32b8cbf8

1. +++ ->
fprintf( stderr, "+++ alloced tmpString (0x%lx) of size uallocated (%ld)\n", tmpString, STRING_SIZE(stringPtr->uallocated) );
just before
stringPtr = tmpString;

2. +++ ->
fprintf( stderr, "+++ copying %ld bytes from 0x%lx to 0x%lx\n", appendNumChars * sizeof(Tcl_UniChar), unicode, (VOID *)(stringPtr->unicode + stringPtr->numChars));
just before the memcpy() call.

-- Matthias Kraft

Discussion

1 2 3 > >> (Page 1 of 3)
  • Matthias Kraft
    Matthias Kraft
    2009-01-08

    Stack trace on AIX with Tcl 8.4.19:

    Segmentation fault in . at 0xf050
    0x0000f050 warning: Unable to access address 0xf050 from core
    (dbx) where
    warning: Unable to access address 0xf050 from core
    warning: Unable to access address 0xf050 from core
    warning: Unable to access address 0xf04c from core
    warning: Unable to access address 0xf04c from core
    warning: Unable to access address 0xf050 from core
    warning: Unable to access address 0xf050 from core
    warning: Unable to access address 0xf04c from core
    warning: Unable to access address 0xf04c from core
    warning: Unable to access address 0xf050 from core
    .() at 0xf050
    AppendUnicodeToUnicodeRep(objPtr = 0x20996ca0, unicode = 0x20b59ff8, appendNumChars = 4096), line 1260 in "tclStringObj.c"
    Tcl_AppendObjToObj(objPtr = 0x20996ca0, appendObjPtr = 0x209967c0), line 1154 in "tclStringObj.c"
    TclPtrSetVar(interp = 0x20090e08, varPtr = 0x2ff21718, arrayPtr = (nil), part1 = warning: Unable to access address 0x20b24ff8 from core
    (invalid char ptr (0x20b24ff8)), part2 = (nil), newValuePtr = 0x209967c0, flags = 516), line 1666 in "tclVar.c"
    TclExecuteByteCode(interp = 0x20090e08, codePtr = 0x20b46eb0), line 2072 in "tclExecute.c"
    TclCompEvalObj(interp = 0x20090e08, objPtr = 0x20996958), line 1107 in "tclExecute.c"
    unnamed block $b1240, line 1180 in "tclProc.c"
    TclObjInterpProc(clientData = 0x20aa4fe0, interp = 0x20090e08, objc = 1, objv = 0x2ff21ca8), line 1180 in "tclProc.c"
    TclEvalObjvInternal(interp = 0x20090e08, objc = 1, objv = 0x2ff21ca8, command = warning: Unable to access address 0x20a73ff4 from core
    (invalid char ptr (0x20a73ff4)), length = 4, flags = 0), line 3219 in "tclBasic.c"
    Tcl_EvalEx(interp = 0x20090e08, script = warning: Unable to access address 0x20a73d00 from core
    (invalid char ptr (0x20a73d00)), numBytes = 760, flags = 0), line 4011 in "tclBasic.c"
    Tcl_FSEvalFile(interp = 0x20090e08, pathPtr = 0x204b5f70), line 1776 in "tclIOUtil.c"
    Tcl_Main(argc = -1, argv = 0x2ff222c0, appInitProc = 0x2001b92c), line 295 in "tclMain.c"
    main(argc = 2, argv = 0x2ff222b8), line 288 in "instsh.c"

     
  • Matthias Kraft
    Matthias Kraft
    2009-01-08

    Stack trace on Linux x86_64 with Tcl 8.5.6:

    Program terminated with signal 11, Segmentation fault.
    #0 0x00007f5443b0ddd3 in memcpy () from /lib64/libc.so.6
    (gdb) bt
    #0 0x00007f5443b0ddd3 in memcpy () from /lib64/libc.so.6
    #1 0x00007f544433f23e in AppendUnicodeToUnicodeRep (objPtr=0x624330, unicode=0x63a55c, appendNumChars=4096)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1336
    #2 0x00007f544433efb3 in Tcl_AppendObjToObj (objPtr=0x624330, appendObjPtr=0x6244e0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1230
    #3 0x00007f54443536e1 in TclPtrSetVar (interp=0x605790, varPtr=0x6067b0, arrayPtr=0x0, part1Ptr=0x0, part2Ptr=0x0, newValuePtr=0x6244e0, flags=516, index=1)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclVar.c:1881
    #4 0x00007f54442deed5 in TclExecuteByteCode (interp=0x605790, codePtr=0x6367f0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclExecute.c:2952
    #5 0x00007f5444337011 in TclObjInterpProcCore (interp=0x605790, procNameObj=0x623b20, skip=1, errorProc=0x7f544433771f <MakeProcError>)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclProc.c:1748
    #6 0x00007f5444336f0b in TclObjInterpProc (clientData=0x6272f0, interp=0x605790, objc=1, objv=0x6065c0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclProc.c:1642
    #7 0x00007f5444279c39 in TclEvalObjvInternal (interp=0x605790, objc=1, objv=0x6065c0, command=0x634b46 "bar\n", length=4, flags=0)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:3690
    #8 0x00007f544427af60 in TclEvalEx (interp=0x605790,
    script=0x6347d0 "set i 1234567890\nset n /dev/zero\n\nproc foo {n i} {\n set f [open $n r]\n fconfigure $f -translation binary -buffering full -buffersize 4096\n\n upvar #0 data($n,raw) r\n set r \"\"\n\n while {$i"..., numBytes=890, flags=0, line=46) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:4338
    #9 0x00007f544427a4b5 in Tcl_EvalEx (interp=0x605790,
    script=0x6347d0 "set i 1234567890\nset n /dev/zero\n\nproc foo {n i} {\n set f [open $n r]\n fconfigure $f -translation binary -buffering full -buffersize 4096\n\n upvar #0 data($n,raw) r\n set r \"\"\n\n while {$i"..., numBytes=890, flags=0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclBasic.c:4043
    #10 0x00007f544430e3e1 in Tcl_FSEvalFileEx (interp=0x605790, pathPtr=0x624690, encodingName=0x0) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclIOUtil.c:1814
    #11 0x00007f5444316ed0 in Tcl_Main (argc=-1, argv=0x7fff4c7ba4a8, appInitProc=0x400925 <Tcl_AppInit>) at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclMain.c:441
    #12 0x000000000040091e in main (argc=2, argv=0x7fff4c7ba498) at /home/makr/cvs/tccvs/tcl-8.5/unix/../unix/tclAppInit.c:87

     
  • Matthias Kraft
    Matthias Kraft
    2009-01-08

    As I now have missed the bus anyway, there is time to ask for a patch that could be ported back to 8.4 :-) ...

     
  • Matthias Kraft
    Matthias Kraft
    2009-01-08

    • priority: 5 --> 9
     
  • Do you mean you have confidence that the bug is no longer present after 8.4 ?
    What are your reasons to stick to 8.4 ?

     
  • Matthias Kraft
    Matthias Kraft
    2009-01-08

    This bug is present in Tcl 8.5.6, but I have not the freedom of choice and am still stuck at 8.4.19. So if someone can fix it for 8.5 I kindly ask to provide a patch that could be applied against 8.4, too, or back ported.

     
  • Don Porter
    Don Porter
    2009-01-08

    if this is fixed reasonably quickly,
    the fix will also be part of 8.4.20.

     
  • Don Porter
    Don Porter
    2009-01-08

    In case it's not already obvious,
    the crash happens when "numChars"
    value escapes the range of a signed int.

     
  • Don Porter
    Don Porter
    2009-01-08

    sorry, I think my hexadigit counting
    was off by one. The crash happens
    reallocating for 2**28 characters.

     
  • Don Porter
    Don Porter
    2009-01-08

    Found it. Problem is in the STRING_UALLOC() macro:

    #define STRING_UALLOC(numChars) \ (numChars * sizeof(Tcl_UniChar))

    It needs changing to:

    #define STRING_UALLOC(numChars) \ ((numChars) * sizeof(Tcl_UniChar))

     
  • Don Porter
    Don Porter
    2009-01-08

    fixed in all branches

     
  • Don Porter
    Don Porter
    2009-01-08

    • assigned_to: msofer --> dgp
    • status: open --> closed-fixed
     
  • Matthias Kraft
    Matthias Kraft
    2009-01-09

    • status: closed-fixed --> open-fixed
     
  • Matthias Kraft
    Matthias Kraft
    2009-01-09

    Sorry Don, but it is not fixed. I tested core-8.5-branch with your changes this morning and it still dumps core on Linux at the same place with the same stack trace ...

     
  • Matthias Kraft
    Matthias Kraft
    2009-01-09

    Testing the patch with my customized Tcl 8.4.19 on AIX, shows that it seems to be fixed there ... that means I am running into out-of-memory now and up to that point the buffers seem to be ok ...

     
  • Matthias Kraft
    Matthias Kraft
    2009-01-09

    ok, looked into 8.5 again: it now seems to be some kind of integer overflow, or something in this direction.

    My Linux build is 64 bit, that means sizeof(size_t) is 8, while sizeof(int) is 4.

    The core dump now shows:

    - the memcpy() call in AppendUnicodeToUnicodeep() crashes again:
    #0 0x00007febad876dd3 in memcpy () from /lib64/libc.so.6
    #1 0x00007febae0a82d1 in AppendUnicodeToUnicodeRep (objPtr=0x625990, unicode=0x63bafc, appendNumChars=4096)
    at /home/makr/cvs/tccvs/tcl-8.5/unix/../generic/tclStringObj.c:1336

    - the other values are:
    (gdb) print *stringPtr
    $2 = {numChars = 1073737728, allocated = 16384, uallocated = 4294967296, hasUnicode = 1, unicode = {222, 173}}
    (gdb) print numChars
    $3 = 1073741824

    - $3 == 1<<30 == 1GB

    - (gdb) print numChars - stringPtr->numChars
    $6 = 4096

    - stringPtr->uallocated == 1<<32 == 4GB which means the attemptckrealloc() has not failed

    so far my observations, maybe I have time to come back to it later today ...

     
  • Don Porter
    Don Porter
    2009-01-09

    trouble in STRING_SIZE() also fixed
    in all branches.

     
  • Don Porter
    Don Porter
    2009-01-09

    • status: open-fixed --> closed-fixed
     
  • Matthias Kraft
    Matthias Kraft
    2009-01-15

    From a mail I sent to the tcl-core list:

    I want to give two points to your attention that concern me with the fix of this bug. But first thank you very much for helping to find the problem and then fixing it promptly!

    1) The fix as now implemented restricts the content of variables to INT_MAX/4 which is 512MB! To my mind this is comparatively small and should be documented somewhere. Please notice that this is also half the size, of what was possible in 8.4.19 / 8.5.6.

    2) In tclStringObj.c is a lengthy documentation of the memory allocation algorithm for strings. This is now obsolete in the case when more than INT_MAX/4 memory is free and could be allocated. As in this case we panic() before even coming to the attemptckalloc().

    What you should make with this information? Well, I don't know. I just wanted to make sure it doesn't get lost and someone has to learn it the hard way. I can live with the fix now.

    Where is the use case that needs so much memory? Consider vfs::memchan. This is where I stumbled over the issue. I thought I use vfs::memchan to be able to transparently uncompress and evaluate the crc of archives...

     
  • Don Porter
    Don Porter
    2009-01-15

    • priority: 9 --> 6
    • status: closed-fixed --> open
     
  • Don Porter
    Don Porter
    2009-01-21

    matzek, can you provide your
    evidence that the commit has
    dropped the max value size
    in half?

    Thanks for pointing out
    the problem with attempt.
    The new panic is in the wrong
    place. Trickier than I thought.

     
  • Don Porter
    Don Porter
    2009-01-21

    ok, I think fixing the
    panic on attempt problem corrects
    both issues. When the
    attemp panicked prematurely,
    that cut off the "slow growth"
    portion of the string growth algorithm,
    which would in fact reduce the allocatable
    buffer by about a factor two.

    Patch on the way...

     
  • Don Porter
    Don Porter
    2009-01-21

    fix committed to HEAD.

    Please test and after you
    confirm success, I will backport.

     
  • Don, please review your changes. On HPUX IA64 I am now getting errors and these are in the macros you modified. When I diffed to history I saw that they became quite complex.

    andreask@bigsur03:~/workbench/z> tail log/tcl-8.6/make.log
    Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2720 # Illegal types found in conditional operator: 'long' and 'char *'.
    copyStringPtr = stringAlloc(STRING_UALLOC(0));
    ^^^^^^^^^^^
    Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2723 # Illegal types found in conditional operator: 'long' and 'char *'.
    copyStringPtr = stringAlloc(srcStringPtr->uallocated);
    ^^^^^^^^^^^
    Error 252: "/home/andreask/workbench/z/tcl-8.6/generic/tclStringObj.c", line 2789 # Illegal types found in conditional operator: 'long' and 'char *'.
    stringPtr = stringAlloc(STRING_UALLOC(0));
    ^^^^^^^^^^^
    make: *** [tclStringObj.o] Error 2
    andreask@bigsur03:~/workbench/z>

     
  • Fix: The NULL after the Tcl_Panic in STRING_NOMEM needs a cast to (char*).

     
1 2 3 > >> (Page 1 of 3)