heap-buffer-overflow in yyl_try

[dur-randir]

This is a bug report for perl from [email protected],
generated with the help of perlbug 1.41 running under perl 5.31.10.

[Please describe your issue here]

While fuzzing perl v5.31.9-70-g0c96aa4b7b built with afl and run
under libdislocator, I found the following program

m�$0[
==0�

00000000  6d 7f 24 30 5b 0a 3d 3d  30 7f                    |m.$0[.==0.|

to cause heap-buffer-overflow. ASAN diagnostics are:

READ of size 5 at 0x602000001298 thread T0
#0 0x4d9bb2 in __interceptor_memcmp.part.302 (/home/afl/afl-asan/perl+0x4d9bb2)
#1 0x6bc842 in yyl_try /home/afl/afl-asan/toke.c:8838:16
#2 0x69573d in Perl_yylex /home/afl/afl-asan/toke.c:9406:19
#3 0x752c37 in Perl_yyparse /home/afl/afl-asan/perly.c:340:34
#4 0x61a38c in S_parse_body /home/afl/afl-asan/perl.c:2579:9
#5 0x610246 in perl_parse /home/afl/afl-asan/perl.c:1870:2
#6 0x5352bd in main /home/afl/afl-asan/perlmain.c:132:18
#7 0x7f51c6b4709a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
#8 0x43ccb9 in _start (/home/afl/afl-asan/perl+0x43ccb9)

0x602000001298 is located 0 bytes to the right of 8-byte region [0x602000001290,0x602000001298)
allocated by thread T0 here:
#0 0x501f10 in realloc (/home/afl/afl-asan/perl+0x501f10)
#1 0x8f6127 in Perl_safesysrealloc /home/afl/afl-asan/util.c:279:18
#2 0x6e6e0c in Perl_scan_str /home/afl/afl-asan/toke.c:11322:2
#3 0x74220c in S_scan_pat /home/afl/afl-asan/toke.c:10214:9
#4 0x725c51 in yyl_word_or_keyword /home/afl/afl-asan/toke.c:7997:13
#5 0x6fc172 in yyl_keylookup /home/afl/afl-asan/toke.c:8633:12
#6 0x6beb1a in yyl_try /home/afl/afl-asan/toke.c
#7 0x702395 in yyl_fake_eof /home/afl/afl-asan/toke.c
#8 0x6bf99e in yyl_try /home/afl/afl-asan/toke.c:8753:16
#9 0x69573d in Perl_yylex /home/afl/afl-asan/toke.c:9406:19
#10 0x752c37 in Perl_yyparse /home/afl/afl-asan/perly.c:340:34
#11 0x61a38c in S_parse_body /home/afl/afl-asan/perl.c:2579:9
#12 0x610246 in perl_parse /home/afl/afl-asan/perl.c:1870:2
#13 0x5352bd in main /home/afl/afl-asan/perlmain.c:132:18
#14 0x7f51c6b4709a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)

This is regression in blead, bisect points to the following commit:

0ae5281 is the first bad commit

commit 0ae5281a2d3f28e842a78e9faed15943ed222e70
Author: Aaron Crane <[email protected]>
Date:   Sun Oct 20 11:35:57 2019 +0200

    toke.c: simplify conflict-marker detection

    Since doing this involves a `goto`, hoisting the check to the top of each
    relevant `case` will make it easier to perform further refactoring.

[Please do not change anything below this line]
Flags:
category=core
severity=high
Site configuration information for perl 5.31.10:

Configured by root at Fri Mar 13 17:15:02 MSK 2020.

Summary of my perl5 (revision 5 version 31 subversion 10) configuration:
Commit id: 0c96aa4
Platform:
osname=linux
osvers=4.19.0-8-amd64
archname=x86_64-linux
uname='linux dorothy 4.19.0-8-amd64 #1 smp debian 4.19.98-1 (2020-01-26) x86_64 gnulinux '
config_args='-de -Dusedevel -Doptimize=-O2'
hint=recommended
useposix=true
d_sigaction=define
useithreads=undef
usemultiplicity=undef
use64bitint=define
use64bitall=define
uselongdouble=undef
usemymalloc=n
default_inc_excludes_dot=define
bincompat5005=undef
Compiler:
cc='cc'
ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
optimize='-O2'
cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
ccversion=''
gccversion='8.3.0'
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='cc'
ldflags =' -fstack-protector-strong -L/usr/local/lib'
libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/8/include-fixed /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.28.so
so=so
useshrplib=false
libperl=libperl.a
gnulibc_version='2.28'
Dynamic Linking:
dlsrc=dl_dlopen.xs
dlext=so
d_dlsymun=undef
ccdlflags='-Wl,-E'
cccdlflags='-fPIC'
lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'

@inc for perl 5.31.10:
lib
/usr/local/lib/perl5/site_perl/5.31.10/x86_64-linux
/usr/local/lib/perl5/site_perl/5.31.10
/usr/local/lib/perl5/5.31.10/x86_64-linux
/usr/local/lib/perl5/5.31.10

Environment for perl 5.31.10:
HOME=/home/afl
LANG=en_US.UTF-8
LANGUAGE=en_US:en
LC_CTYPE=en_US.UTF-8
LC_TIME=C
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.30.0-dbg/bin:/opt/local/bin:/usr/texbin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PERLBREW_HOME=/home/afl/.perlbrew
PERLBREW_MANPATH=/home/afl/perlbrew/perls/perl-5.30.0-dbg/man
PERLBREW_PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.30.0-dbg/bin
PERLBREW_PERL=perl-5.30.0-dbg
PERLBREW_ROOT=/home/afl/perlbrew
PERLBREW_SHELLRC_VERSION=0.88
PERLBREW_VERSION=0.88
PERL_BADLANG (unset)

[hvds]

The \x7f aren't needed, so it can be reproduced with perl -wle 'print qq{m/\$0[\n==0/}' | ./miniperl.

@arc in the fingered commit 0ae5281, the case '<' carefully allows for the adjusted s by replacing it with (s+2); in the other two cases it fails to do so, so I think it needs this fix:

--- a/toke.c
+++ b/toke.c
@@ -8835,7 +8835,7 @@ yyl_try(pTHX_ char *s, STRLEN len)
 
     case '=':
         if (s[1] == '=' && (s == PL_linestart || s[-1] == '\n')
-            && memBEGINs(s + 2, (STRLEN) (PL_bufend - s + 2), "====="))
+            && memBEGINs(s + 2, (STRLEN) (PL_bufend - (s + 2)), "====="))
         {
             s = vcs_conflict_marker(s + 7);
             goto retry;
@@ -8938,7 +8938,7 @@ yyl_try(pTHX_ char *s, STRLEN len)
 
     case '>':
         if (s[1] == '>' && (s == PL_linestart || s[-1] == '\n')
-            && memBEGINs(s + 2, (STRLEN) (PL_bufend - s + 2), ">>>>>"))
+            && memBEGINs(s + 2, (STRLEN) (PL_bufend - (s + 2)), ">>>>>"))
         {
             s = vcs_conflict_marker(s + 7);
             goto retry;

That's enough to make asan quiet; if someone can suggest how to write a test for this I'm happy to finish it off.

[tonycoz]

That's enough to make asan quiet; if someone can suggest how to write a test for this I'm happy to finish it off.

Typically we way we test this type of thing it to use fresh_perl_is() to test that the output of the error is as expected, ie. doesn't include the ASAN error. Adding a test to t/lib/croak/toke

It has the weakness that it only fails on ASAN/valgrind, but I don't see anything else we can do for cases like this.

I normally include a comment indicating that the test only failed under ASAN/valgrind.

[hvds]

Thanks @tonycoz, and apologies for the delay. Now fixed with e0ab3ef (test) and 51c9ef5 (fix).

Will close after some smoke results.