Conditional jump depends on uninitialized values in S_scan_heredoc

[p5pRT]

Migrated from rt.perl.org#129176 (status was 'resolved')

Searchable as RT129176$

[p5pRT]

From @dcollinsn

$ perl -e 'print "<<`\\"' | valgrind ../bin/perl
==20392== Conditional jump or move depends on uninitialised value(s)
==20392== at 0x4C2D67A​: __GI_strchr (vg_replace_strmem.c​:246)
==20392== by 0x4DA7A9​: S_scan_heredoc (toke.c​:9590)
==20392== by 0x4DA7A9​: Perl_yylex (toke.c​:6216)
==20392== by 0x4F7C05​: Perl_yyparse (perly.c​:334)
==20392== by 0x479F38​: S_parse_body (perl.c​:2373)
==20392== by 0x479F38​: perl_parse (perl.c​:1689)
==20392== by 0x41FCB2​: main (perlmain.c​:121)
==20392==
==20392== Conditional jump or move depends on uninitialised value(s)
==20392== at 0x4C2D680​: __GI_strchr (vg_replace_strmem.c​:246)
==20392== by 0x4DA7A9​: S_scan_heredoc (toke.c​:9590)
==20392== by 0x4DA7A9​: Perl_yylex (toke.c​:6216)
==20392== by 0x4F7C05​: Perl_yyparse (perly.c​:334)
==20392== by 0x479F38​: S_parse_body (perl.c​:2373)
==20392== by 0x479F38​: perl_parse (perl.c​:1689)
==20392== by 0x41FCB2​: main (perlmain.c​:121)
==20392==
Can't find string terminator "\" anywhere before EOF at - line 1.
==20392==
==20392== HEAP SUMMARY​:
==20392== in use at exit​: 144,138 bytes in 535 blocks
==20392== total heap usage​: 688 allocs, 153 frees, 185,856 bytes allocated
==20392==
==20392== LEAK SUMMARY​:
==20392== definitely lost​: 7,590 bytes in 15 blocks
==20392== indirectly lost​: 136,548 bytes in 520 blocks
==20392== possibly lost​: 0 bytes in 0 blocks
==20392== still reachable​: 0 bytes in 0 blocks
==20392== suppressed​: 0 bytes in 0 blocks
==20392== Rerun with --leak-check=full to see details of leaked memory
==20392==
==20392== For counts of detected and suppressed errors, rerun with​: -v
==20392== Use --track-origins=yes to see where uninitialised values come from
==20392== ERROR SUMMARY​: 2 errors from 2 contexts (suppressed​: 0 from 0)

I'll also include a disassemble, to highlight that this is happening in some SSE type instructions​:

dcollins@​nightshade64​:/usr/local/perl-afl/out$ LD_PRELOAD=/home/dcollins/toolcha in/afl-2.32b/libdislocator/libdislocator.so gdb --args ../bin/perl allcrash/f1i0 00000
GNU gdb (Debian 7.11.1-2) 7.11.1
Copyright (C) 2016 Free Software Foundation, Inc.
License GPLv3+​: GNU GPL version 3 or later <http​://gnu.org/licenses/gpl.html>
This is free software​: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see​:
<http​://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at​:
<http​://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ../bin/perl...done.
(gdb) run
Starting program​: /usr/local/perl-afl/bin/perl allcrash/f1i000000
b[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Program received signal SIGSEGV, Segmentation fault.
__strchr_sse2 () at ../sysdeps/x86_64/multiarch/../strchr.S​:87
87 ../sysdeps/x86_64/multiarch/../strchr.S​: No such file or directory.
(gdb) bt
#0 __strchr_sse2 () at ../sysdeps/x86_64/multiarch/../strchr.S​:87
#1 0x00000000004da7aa in S_scan_heredoc (my_perl=0x7ffff7ff3258,
  s=0x7ffff65c0ffc "AAAA") at toke.c​:9590
#2 Perl_yylex (my_perl=<optimized out>) at toke.c​:6216
#3 0x00000000004f7c06 in Perl_yyparse (my_perl=<optimized out>,
  gramtype=<optimized out>) at perly.c​:334
#4 0x0000000000479f39 in S_parse_body (my_perl=<optimized out>,
  env=<optimized out>, xsinit=<optimized out>) at perl.c​:2373
#5 perl_parse (my_perl=<optimized out>, xsinit=<optimized out>,
  argc=<optimized out>, argv=<optimized out>, env=<optimized out>)
  at perl.c​:1689
#6 0x000000000041fcb3 in main (argc=-161738816, argv=0xf65c0f3c,
  env=<optimized out>) at perlmain.c​:121
(gdb) exploitable
Description​: Access violation on source operand
Short description​: SourceAv (19/22)
Hash​: 3cda16a9d29df0c5bcdcdcd95093363c.9c89f70cf23d6b4b31419a8bd7bf434f
Exploitability Classification​: UNKNOWN
Explanation​: The target crashed on an access violation at an address matching the source operand of the current instruction. This likely indicates a read access violation.
Other tags​: AccessViolation (21/22)
(gdb) disassemble
Dump of assembler code for function __strchr_sse2​:
  0x00007ffff6b3a2e0 <+0>​: movd xmm1,esi
  0x00007ffff6b3a2e4 <+4>​: mov eax,edi
  0x00007ffff6b3a2e6 <+6>​: and eax,0xfff
  0x00007ffff6b3a2eb <+11>​: punpcklbw xmm1,xmm1
  0x00007ffff6b3a2ef <+15>​: cmp eax,0xfc0
  0x00007ffff6b3a2f4 <+20>​: punpcklwd xmm1,xmm1
  0x00007ffff6b3a2f8 <+24>​: pshufd xmm1,xmm1,0x0
  0x00007ffff6b3a2fd <+29>​: jg 0x7ffff6b3a460 <__strchr_sse2+384>
  0x00007ffff6b3a303 <+35>​: movdqu xmm0,XMMWORD PTR [rdi]
  0x00007ffff6b3a307 <+39>​: pxor xmm3,xmm3
  0x00007ffff6b3a30b <+43>​: movdqa xmm4,xmm0
  0x00007ffff6b3a30f <+47>​: pcmpeqb xmm0,xmm1
  0x00007ffff6b3a313 <+51>​: pcmpeqb xmm4,xmm3
  0x00007ffff6b3a317 <+55>​: por xmm0,xmm4
  0x00007ffff6b3a31b <+59>​: pmovmskb eax,xmm0
  0x00007ffff6b3a31f <+63>​: test eax,eax
  0x00007ffff6b3a321 <+65>​: je 0x7ffff6b3a338 <__strchr_sse2+88>
  0x00007ffff6b3a323 <+67>​: bsf eax,eax
  0x00007ffff6b3a326 <+70>​: mov edx,0x0
  0x00007ffff6b3a32b <+75>​: lea rax,[rdi+rax*1]
  0x00007ffff6b3a32f <+79>​: cmp BYTE PTR [rax],sil
  0x00007ffff6b3a332 <+82>​: cmovne rax,rdx
  0x00007ffff6b3a336 <+86>​: ret
  0x00007ffff6b3a337 <+87>​: nop
  0x00007ffff6b3a338 <+88>​: movdqu xmm0,XMMWORD PTR [rdi+0x10]
  0x00007ffff6b3a33d <+93>​: movdqa xmm4,xmm0
  0x00007ffff6b3a341 <+97>​: pcmpeqb xmm0,xmm1
  0x00007ffff6b3a345 <+101>​: pcmpeqb xmm4,xmm3
  0x00007ffff6b3a349 <+105>​: por xmm0,xmm4
  0x00007ffff6b3a34d <+109>​: pmovmskb ecx,xmm0
  0x00007ffff6b3a351 <+113>​: movdqu xmm0,XMMWORD PTR [rdi+0x20]
  0x00007ffff6b3a356 <+118>​: movdqa xmm4,xmm0
  0x00007ffff6b3a35a <+122>​: pcmpeqb xmm0,xmm1
  0x00007ffff6b3a35e <+126>​: shl rcx,0x10
  0x00007ffff6b3a362 <+130>​: pcmpeqb xmm4,xmm3
  0x00007ffff6b3a366 <+134>​: por xmm0,xmm4
  0x00007ffff6b3a36a <+138>​: pmovmskb eax,xmm0
  0x00007ffff6b3a36e <+142>​: movdqu xmm0,XMMWORD PTR [rdi+0x30]
  0x00007ffff6b3a373 <+147>​: pcmpeqb xmm3,xmm0
  0x00007ffff6b3a377 <+151>​: shl rax,0x20
  0x00007ffff6b3a37b <+155>​: pcmpeqb xmm0,xmm1
  0x00007ffff6b3a37f <+159>​: or rax,rcx
  0x00007ffff6b3a382 <+162>​: por xmm0,xmm3
  0x00007ffff6b3a386 <+166>​: pmovmskb ecx,xmm0
  0x00007ffff6b3a38a <+170>​: shl rcx,0x30
  0x00007ffff6b3a38e <+174>​: or rax,rcx
  0x00007ffff6b3a391 <+177>​: test rax,rax
  0x00007ffff6b3a394 <+180>​: jne 0x7ffff6b3a440 <__strchr_sse2+352>
  0x00007ffff6b3a39a <+186>​: nop WORD PTR [rax+rax*1+0x0]
  0x00007ffff6b3a3a0 <+192>​: pxor xmm6,xmm6
  0x00007ffff6b3a3a4 <+196>​: and rdi,0xffffffffffffffc0
  0x00007ffff6b3a3a8 <+200>​: add rdi,0x40
=> 0x00007ffff6b3a3ac <+204>​: movdqa xmm5,XMMWORD PTR [rdi]
  0x00007ffff6b3a3b0 <+208>​: movdqa xmm2,XMMWORD PTR [rdi+0x10]
  0x00007ffff6b3a3b5 <+213>​: movdqa xmm3,XMMWORD PTR [rdi+0x20]
  0x00007ffff6b3a3ba <+218>​: pxor xmm5,xmm1
  0x00007ffff6b3a3be <+222>​: movdqa xmm4,XMMWORD PTR [rdi+0x30]
  0x00007ffff6b3a3c3 <+227>​: pxor xmm2,xmm1
  0x00007ffff6b3a3c7 <+231>​: pxor xmm3,xmm1
  0x00007ffff6b3a3cb <+235>​: pminub xmm5,XMMWORD PTR [rdi]
  0x00007ffff6b3a3cf <+239>​: pxor xmm4,xmm1
  0x00007ffff6b3a3d3 <+243>​: pminub xmm2,XMMWORD PTR [rdi+0x10]
  0x00007ffff6b3a3d8 <+248>​: pminub xmm3,XMMWORD PTR [rdi+0x20]
  0x00007ffff6b3a3dd <+253>​: pminub xmm5,xmm2
  0x00007ffff6b3a3e1 <+257>​: pminub xmm4,XMMWORD PTR [rdi+0x30]
  0x00007ffff6b3a3e6 <+262>​: pminub xmm5,xmm3
  0x00007ffff6b3a3ea <+266>​: pminub xmm5,xmm4
  0x00007ffff6b3a3ee <+270>​: pcmpeqb xmm5,xmm6
  0x00007ffff6b3a3f2 <+274>​: pmovmskb eax,xmm5
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb)

I'll poke at it when I'm less starving to see whether it's dependent on a certain -O level. This appears to be unrelated to the previous crashes in this function, namely 123712, 126815, 125540, because all of those have been fixed ;) Here's my perl​:

Summary of my perl5 (revision 5 version 25 subversion 5) configuration​:
  Commit id​: 483efd0
  Platform​:
  osname=linux
  osvers=4.6.0-1-amd64
  archname=x86_64-linux-thread-multi
  uname='linux nightshade64 4.6.0-1-amd64 #1 smp debian 4.6.4-1 (2016-07-18) x86_64 gnulinux '
  config_args='-Dusedevel -Dusethreads -Duselongdoubles -Dcc=afl-clang-fast -des -Doptimize=-O3 -g3 -Dprefix=/usr/local/perl-afl -Uman1dir -Uman3dir -Uversiononly'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=define
  uselongdouble=undef
  usemymalloc=n
  bincompat5005=undef
  Compiler​:
  cc='afl-clang-fast'
  ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
  optimize='-O3 -g3'
  cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='4.2.1 Compatible Clang 3.9.0 (trunk 273094)'
  gccosandvers=''
  intsize=4
  longsize=8
  ptrsize=8
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=16
  longdblkind=3
  ivtype='long'
  ivsize=8
  nvtype='double'
  nvsize=8
  Off_t='off_t'
  lseeksize=8
  alignbytes=8
  prototype=define
  Linker and Libraries​:
  ld='afl-clang-fast'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.23.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.23'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O3 -g3 -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  MULTIPLICITY
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD
  PERL_IMPLICIT_CONTEXT
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_ITHREADS
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_PERLIO
  USE_PERL_ATOF
  USE_REENTRANT_API
  Built under linux
  Compiled at Sep 1 2016 20​:01​:32
  %ENV​:
  PERLBREW_BASHRC_VERSION="0.76"
  PERLBREW_HOME="/home/dcollins/.perlbrew"
  PERLBREW_ROOT="/home/dcollins/toolchain/perl5"
  @​INC​:
  /usr/local/perl-afl/lib/site_perl/5.25.5/x86_64-linux-thread-multi
  /usr/local/perl-afl/lib/site_perl/5.25.5
  /usr/local/perl-afl/lib/5.25.5/x86_64-linux-thread-multi
  /usr/local/perl-afl/lib/5.25.5
  .

--
Respectfully,
Dan Collins

[p5pRT]

From @geeknik

I believe I already reported this to the security queue. 😆

On Friday, September 2, 2016, Dan Collins <perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Dan Collins
# Please include the string​: [perl #129176]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129176 >

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @cpansprout

On Fri Sep 02 13​:38​:21 2016, dcollinsn@​gmail.com wrote​:

$ perl -e 'print "<<`\\"' | valgrind ../bin/perl
...
Can't find string terminator "\" anywhere before EOF at - line 1.

Wrong error message.

delimcpy (used to find the end of the `\... after <<) is going one byte past the end of the buffer passed to it, because the trailing null is ‘escaped’ with a backslash. delimcpy needs fixing.

scan_heredoc perfectly reasonably croaks only if s==PL_bufend, since if s<PL_bufend the terminating ` was found. In this case s>PL_bufend, which would not happen with a properly functioning delimcpy.

--

Father Chrysostomos

[p5pRT]

From @iabyn

On Fri, Sep 02, 2016 at 10​:31​:37PM -0700, Father Chrysostomos via RT wrote​:

On Fri Sep 02 13​:38​:21 2016, dcollinsn@​gmail.com wrote​:

$ perl -e 'print "<<`\\"' | valgrind ../bin/perl
...
Can't find string terminator "\" anywhere before EOF at - line 1.

Wrong error message.

delimcpy (used to find the end of the `\... after <<) is going one byte past the end of the buffer passed to it, because the trailing null is ‘escaped’ with a backslash. delimcpy needs fixing.

scan_heredoc perfectly reasonably croaks only if s==PL_bufend, since if s<PL_bufend the terminating ` was found. In this case s>PL_bufend, which would not happen with a properly functioning delimcpy.

Note that I already have a fix worked up for this; I can't apply it yet
since it applies on top of another fix by Tony which hasn't been applied
yet (which was also an issue initially reported to the security queue but
which was provisionally agreed not to be a security issue).

--
You live and learn (although usually you just live).

[p5pRT]

From @cpansprout

On Mon Sep 05 06​:13​:44 2016, davem wrote​:

On Fri, Sep 02, 2016 at 10​:31​:37PM -0700, Father Chrysostomos via RT
wrote​:

On Fri Sep 02 13​:38​:21 2016, dcollinsn@​gmail.com wrote​:

$ perl -e 'print "<<`\\"' | valgrind ../bin/perl
...
Can't find string terminator "\" anywhere before EOF at - line 1.

Wrong error message.

delimcpy (used to find the end of the `\... after <<) is going one
byte past the end of the buffer passed to it, because the trailing
null is ‘escaped’ with a backslash. delimcpy needs fixing.

scan_heredoc perfectly reasonably croaks only if s==PL_bufend, since
if s<PL_bufend the terminating ` was found. In this case
s>PL_bufend, which would not happen with a properly functioning
delimcpy.

Note that I already have a fix worked up for this;

It doesn’t conflict with ba0a415, does it?

--

Father Chrysostomos

[p5pRT]

From @iabyn

On Mon, Sep 05, 2016 at 06​:41​:12AM -0700, Father Chrysostomos via RT wrote​:

On Mon Sep 05 06​:13​:44 2016, davem wrote​:

On Fri, Sep 02, 2016 at 10​:31​:37PM -0700, Father Chrysostomos via RT
wrote​:

On Fri Sep 02 13​:38​:21 2016, dcollinsn@​gmail.com wrote​:

$ perl -e 'print "<<`\\"' | valgrind ../bin/perl
...
Can't find string terminator "\" anywhere before EOF at - line 1.

Wrong error message.

delimcpy (used to find the end of the `\... after <<) is going one
byte past the end of the buffer passed to it, because the trailing
null is ‘escaped’ with a backslash. delimcpy needs fixing.

scan_heredoc perfectly reasonably croaks only if s==PL_bufend, since
if s<PL_bufend the terminating ` was found. In this case
s>PL_bufend, which would not happen with a properly functioning
delimcpy.

Note that I already have a fix worked up for this;

It doesn’t conflict with ba0a415, does it?

Almost certainly :-)
I'll work round it.

--
Never work with children, animals, or actors.

[p5pRT]

From @cpansprout

On Mon Sep 05 07​:45​:59 2016, davem wrote​:

On Mon, Sep 05, 2016 at 06​:41​:12AM -0700, Father Chrysostomos via RT wrote​:

On Mon Sep 05 06​:13​:44 2016, davem wrote​:

On Fri, Sep 02, 2016 at 10​:31​:37PM -0700, Father Chrysostomos via RT
It doesn’t conflict with ba0a415, does it?

Almost certainly :-)
I'll work round it.

Let me guess, it’s probably something like​:

- if (allow_escape && *from == '\\') {
+ if (allow_escape && from+1 < fromend *from == '\\') {

--

Father Chrysostomos

[p5pRT]

From @iabyn

On Mon, Sep 05, 2016 at 09​:36​:54AM -0700, Father Chrysostomos via RT wrote​:

On Mon Sep 05 07​:45​:59 2016, davem wrote​:

On Mon, Sep 05, 2016 at 06​:41​:12AM -0700, Father Chrysostomos via RT wrote​:

On Mon Sep 05 06​:13​:44 2016, davem wrote​:

On Fri, Sep 02, 2016 at 10​:31​:37PM -0700, Father Chrysostomos via RT
It doesn’t conflict with ba0a415, does it?

Almost certainly :-)
I'll work round it.

Let me guess, it’s probably something like​:

- if (allow_escape && *from == '\\') {
+ if (allow_escape && from+1 < fromend *from == '\\') {

might be ;-)

(plus tests of course)

--
Indomitable in retreat, invincible in advance, insufferable in victory
  -- Churchill on Montgomery

[p5pRT]

From @iabyn

On Tue, Sep 06, 2016 at 08​:37​:46AM +0100, Dave Mitchell wrote​:

On Mon, Sep 05, 2016 at 09​:36​:54AM -0700, Father Chrysostomos via RT wrote​:

On Mon Sep 05 07​:45​:59 2016, davem wrote​:

On Mon, Sep 05, 2016 at 06​:41​:12AM -0700, Father Chrysostomos via RT wrote​:

On Mon Sep 05 06​:13​:44 2016, davem wrote​:

On Fri, Sep 02, 2016 at 10​:31​:37PM -0700, Father Chrysostomos via RT
It doesn’t conflict with ba0a415, does it?

Almost certainly :-)
I'll work round it.

Let me guess, it’s probably something like​:

- if (allow_escape && *from == '\\') {
+ if (allow_escape && from+1 < fromend *from == '\\') {

might be ;-)

(plus tests of course)

Now fixed with 19e1655.

I took the liberty of also doing this​:

  commit 31ee10f
  Author​: David Mitchell <davem@​iabyn.com>
  AuthorDate​: Wed Sep 7 20​:57​:01 2016 +0100

  rename S_delimcpy() to S_delimcpy_intern()
 
  Its a bit confusing having both S_delimcpy() and Perl_delimcpy()
  functions.

--
You're only as old as you look.

[p5pRT]

@iabyn - Status changed from 'open' to 'pending release'

[p5pRT]

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

[p5pRT]

@khwilliamson - Status changed from 'pending release' to 'resolved'