Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

heap-buffer-overflow in yyl_try #17645

Closed
dur-randir opened this issue Mar 13, 2020 · 3 comments
Closed

heap-buffer-overflow in yyl_try #17645

dur-randir opened this issue Mar 13, 2020 · 3 comments

Comments

@dur-randir
Copy link
Member

This is a bug report for perl from sergey.aleynikov@gmail.com,
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 <arc@cpan.org>
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
Copy link
Contributor

hvds commented Mar 13, 2020

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
Copy link
Contributor

tonycoz commented Mar 17, 2020

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 added a commit that referenced this issue Mar 27, 2020
TODO test fails with sanitize=address.
hvds added a commit that referenced this issue Mar 27, 2020
@hvds
Copy link
Contributor

hvds commented Mar 27, 2020

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

Will close after some smoke results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants