develooper Front page | perl.perl5.porters | Postings from October 2018

[perl #133347] BBC SREZIC/Tk-804.034.tar.gz

Thread Previous
From:
James E Keenan via RT
Date:
October 27, 2018 17:15
Subject:
[perl #133347] BBC SREZIC/Tk-804.034.tar.gz
Message ID:
rt-4.0.24-10781-1540660502-1686.133347-15-0@perl.org
On Wed, 12 Sep 2018 16:03:42 GMT, ppisar wrote:
> On 2018-09-11, Karl Williamson <public@khwilliamson.com> wrote:
> > On 09/10/2018 06:00 AM, Petr Pisar wrote:
> >> On 2018-07-08, Karl Williamson <public@khwilliamson.com> wrote:
> >>> I edited encGlue.c to get rid of all these and all the warnings,
> >>> but the
> >>> make test crashes before it gets to where the failures in this
> >>> ticket occur.
> >>>
> >> I came across this issue when I applied:
> >>
> >> commit aa3c16bd709ef9b9c8c785af48f368e08f70c74b
> >> Author: Karl Williamson <khw@cpan.org>
> >> Date:   Tue Jul 17 13:57:54 2018 -0600
> >>
> >> Make utf8_to_uvchr() safer
> >>
> >> This function is deprecated because the API doesn't allow it to
> >> determine the end of the input string, so it can read off the far
> >> end.
> >> But I just realized that since many strings are NUL-terminated, so
> >> we
> >> can forbid it from reading past the next NUL, and hence make it safe
> >> in
> >> many cases.
> >>
> >> to perl-5.28.0. Then unpatched Tk crashed for me in
> >> Tcl_NumUtfChars() at
> >> encGlue.c:117:
> >>
> >> int
> >> Tcl_NumUtfChars(CONST char * src, int len)
> >> {
> >>   U8 *s = (U8 *) src;
> >>   U8 *send;
> >>   if (len < 0)
> >>    len = strlen(src);
> >>   send = s + len;
> >>   len = 0;
> >>   while (s < send)
> >>    {
> >> →  s += UTF8SKIP(s);
> >>     len++;
> >>    }
> >>   return len;
> >> }
> >>
> >> when debugigng Tk-Pod-0.9943 test failure like this:
> >>
> >> $ DISPLAY=:99 perl -Iblib/lib -e 'use Tk::MainWindow;
> >> $m=Tk::MainWindow->new; require Tk::Pod::Search; $m->PodSearch'
> >>
> >> The backtrace was:
> >>
> >> #0  Tcl_NumUtfChars (src=src@entry=0x55a6feecede0 "Search:",
> >> len=25087, len@entry=1853537494) at encGlue.c:117
> >> #1  0x00007f076e750fc4 in NewChunk
> >> (layoutPtrPtr=layoutPtrPtr@entry=0x7ffdcbaaed38,
> >> maxPtr=maxPtr@entry=0x7ffdcbaaed2c, start=start@entry=0x55a6feecede0
> >> "Search:",
> >>      numBytes=numBytes@entry=1853537494, curX=0, newX=9, y=27) at
> >> tkFont.c:3298
> >> #2  0x00007f076e751c93 in Tk_ComputeTextLayout
> >> (tkfont=0x55a6fee8afe0, string=0x55a6feecede0 "Search:",
> >> numChars=<optimized out>, numChars@entry=-1, wrapLength=-1,
> >>      wrapLength@entry=0, justify=justify@entry=TK_JUSTIFY_CENTER,
> >> flags=1, flags@entry=0, widthPtr=0x55a6feebe0c8,
> >> heightPtr=0x55a6feebe0cc) at tkFont.c:1977
> >> #3  0x00007f076e78019e in TkpComputeButtonGeometry
> >> (butPtr=butPtr@entry=0x55a6feebdf50) at tkUnixButton.c:602
> >> #4  0x00007f076e749351 in TkButtonWorldChanged
> >> (instanceData=instanceData@entry=0x55a6feebdf50) at tkButton.c:1392
> >> #5  0x00007f076e749928 in ConfigureButton
> >> (interp=interp@entry=0x55a6fec3aea0,
> >> butPtr=butPtr@entry=0x55a6feebdf50, objc=objc@entry=2,
> >> objv=objv@entry=0x55a6fe685a98)
> >>      at tkButton.c:1279
> >> #6  0x00007f076e74a572 in ButtonWidgetObjCmd
> >> (clientData=clientData@entry=0x55a6feebdf50,
> >> interp=interp@entry=0x55a6fec3aea0, objc=objc@entry=4,
> >> objv=objv@entry=0x55a6fe685a88)
> >>      at tkButton.c:823
> >> #7  0x00007f076e729560 in Call_Tk () at tkGlue.c:2260
> >> #8  0x00007f076e729b4d in XStoWidget (my_perl=0x55a6fe680260,
> >> cv=0x55a6fea63e58) at tkGlue.c:2627
> >> #9  0x00007f076ef91299 in Perl_pp_entersub (my_perl=0x55a6fe680260)
> >> at pp_hot.c:5232
> >> #10 0x00007f076ef87485 in Perl_runops_standard
> >> (my_perl=0x55a6fe680260) at run.c:42
> >> #11 0x00007f076ef03fdd in S_run_body (oldscope=<optimized out>,
> >> my_perl=<optimized out>) at perl.c:2689
> >> #12 perl_run (my_perl=0x55a6fe680260) at perl.c:2617
> >> #13 0x000055a6fd8ab26a in main (argc=<optimized out>,
> >> argv=<optimized out>, env=<optimized out>) at perlmain.c:122
> >>
> >> The "Search:" string 25087 length seems dubious.
> >>
> >> Maybe it's the same reason why your attempt to port Tk to the new
> >> API
> >> ends with a crash.
> >>
> >> -- Petr
> >>
> >
> > Could you run this under valgrind?  I suspect this would show up a
> > bug
> > in the library.
> 
> valgrind reports plenty of jumps depending on an unitilaized memory in
> Tk-Pod XS code on patched and unpatatched perl. The crash happens on
> patched perl randomly. I compared valgrind output between patched and
> unpatched perl and found this new error important:
> 
> ==PID== Use of uninitialised value of size 8
> ==PID==    at ADDRESS: Tcl_NumUtfChars (encGlue.c:117)
> ==PID==    by ADDRESS: NewChunk (tkFont.c:3298)
> ==PID==    by ADDRESS: Tk_ComputeTextLayout (tkFont.c:1977)
> ==PID==    by ADDRESS: TkpComputeButtonGeometry (tkUnixButton.c:602)
> ==PID==    by ADDRESS: TkButtonWorldChanged (tkButton.c:1392)
> ==PID==    by ADDRESS: ConfigureButton (tkButton.c:1279)
> ==PID==    by ADDRESS: ButtonWidgetObjCmd (tkButton.c:823)
> ==PID==    by ADDRESS: Call_Tk (tkGlue.c:2260)
> ==PID==    by ADDRESS: XStoWidget (tkGlue.c:2627)
> ==PID==    by ADDRESS: Perl_pp_entersub (in
> /usr/lib64/libperl.so.5.28.0)
> ==PID==    by ADDRESS: Perl_runops_standard (in
> /usr/lib64/libperl.so.5.28.0)
> ==PID==    by ADDRESS: perl_run (in /usr/lib64/libperl.so.5.28.0)
> 
> The encGlue.c:117 is at:
> 
> int
> Tcl_NumUtfChars(CONST char * src, int len)
> {
>  U8 *s = (U8 *) src;
>  U8 *send;
>  if (len < 0)
>   len = strlen(src);
>  send = s + len;
>  len = 0;
>  while (s < send)
>   {
> →  s += UTF8SKIP(s);
>    len++;
>   }
>  return len;
> }
> 
> It's the same place found by a debugger. I don't think valgrind
> provides any
> other usefull clues. Here is the complete difference if you are
> interested:

[snip -- moved to attachment]

> 
> -- Petr

Could we get an update on the status of this ticket?

khw has mentioned to me:

"These probably indicate potential security flaws, which the maintainer(s) really should investigate.  These are scary, and this code may already be being used as an attack vector."

Speaking of scary ... the other day I attempted to build Tk against perl-5.29.4.  My computer crashed during the test suite.  Twice.

Thank you very much.

-- 
James E Keenan (jkeenan@cpan.org)

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=133347

Thread Previous


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About