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

toke.c:9188: S_parse_ident: Assertion `(((const U8*) (PL_parser->bufend)) > ((const U8*) t) || (((const U8*) (PL_parser->bufend)) == ((const U8*) t) && *((const U8*) t) == '\0'))' failed #15846

Closed
p5pRT opened this issue Jan 29, 2017 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 29, 2017

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

Searchable as RT130666$

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From @dur-randir

Created by @dur-randir

While fuzzing perl v5.25.9-35-g32207c637b built with afl and run
under libdislocator, I found the following 24-bytes program

00000000 42 45 47 49 4e 7b 24 5e 48 3d 2d 31 7d 3b 73 00 |BEGIN{$^H=-1};s.|
00000010 24 30 5b 24 6c 5d 00 00 |$0[$l]..|
00000018

to cause an assertion failure. This is a regression in blead, bisect points to

fac0f7a is the first bad commit
commit fac0f7a
Author​: Karl Williamson <khw@​cpan.org>
Date​: Tue Dec 13 18​:34​:12 2016 -0700

  toke.c​: Convert to use isFOO_utf8_safe() macros

GDB info about the crash location​:

(gdb) bt
#0 __GI_raise (sig=sig@​entry=6) at ../sysdeps/unix/sysv/linux/raise.c​:58
#1 0x00007fb973fc540a in __GI_abort () at abort.c​:89
#2 0x00007fb973fbce47 in __assert_fail_base (fmt=<optimized out>,
  assertion=assertion@​entry=0x7fb975e603f8 "(((const U8*)
(PL_parser->bufend)) > ((const U8*) t) || (((const U8*)
(PL_parser->bufend)) == ((const U8*) t) && *((const U8*) t) ==
'\\0'))", file=file@​entry=0x7fb975e5f0eb "toke.c",
line=line@​entry=9188,
  function=function@​entry=0x7fb975e72428 <__PRETTY_FUNCTION__.19035>
"S_parse_ident") at assert.c​:92
#3 0x00007fb973fbcef2 in __GI___assert_fail (
  assertion=assertion@​entry=0x7fb975e603f8 "(((const U8*)
(PL_parser->bufend)) > ((const U8*) t) || (((const U8*)
(PL_parser->bufend)) == ((const U8*) t) && *((const U8*) t) ==
'\\0'))", file=file@​entry=0x7fb975e5f0eb "toke.c",
line=line@​entry=9188,
  function=function@​entry=0x7fb975e72428 <__PRETTY_FUNCTION__.19035>
"S_parse_ident") at assert.c​:101
#4 0x00007fb975494457 in S_parse_ident (check_dollar=<optimized out>,
is_utf8=<optimized out>, allow_package=<optimized out>, e=<optimized
out>,
  d=<optimized out>, s=<optimized out>) at toke.c​:9188
#5 S_scan_ident (s=<optimized out>, s@​entry=0x7fb977185b23 "$l]",
dest=dest@​entry=0x7ffdc81c17e0 "", destlen=destlen@​entry=1024,
ck_uni=ck_uni@​entry=0)
  at toke.c​:9285
#6 0x00007fb975497583 in S_intuit_more (s=0x7fb977185b23 "$l]") at toke.c​:4171
#7 0x00007fb97551b728 in Perl_yylex () at toke.c​:6590
#8 0x00007fb9754c5d9d in Perl_yylex () at toke.c​:4920
#9 0x00007fb9754dde55 in Perl_yylex () at toke.c​:5016
#10 0x00007fb97555a389 in Perl_yyparse (gramtype=gramtype@​entry=258)
at perly.c​:340
#11 0x00007fb975413131 in S_parse_body (env=env@​entry=0x0,
xsinit=xsinit@​entry=0x7fb975308990 <xs_init>) at perl.c​:2376
#12 0x00007fb975419deb in perl_parse (my_perl=<optimized out>,
xsinit=0x7fb975308990 <xs_init>, argc=<optimized out>, argv=<optimized
out>, env=0x0)
  at perl.c​:1691
#13 0x00007fb97530856e in main (argc=<optimized out>, argv=<optimized
out>, env=<optimized out>) at perlmain.c​:121

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.25.9:

Configured by root at Sat Jan 14 02:25:05 MSK 2017.

Summary of my perl5 (revision 5 version 25 subversion 9) configuration:
  Commit id: cbe2fc5001aa59cdc73e04cc35e097a2ecfbeec0
  Platform:
    osname=linux
    osvers=3.16.0-4-amd64
    archname=x86_64-linux
    uname='linux dorothy 3.16.0-4-amd64 #1 smp debian 3.16.36-1+deb8u2
(2016-10-19) x86_64 gnulinux '
    config_args='-des -Dusedevel -DDEBUGGING -Dcc=afl-clang-fast
-Doptimize=-O0 -g -ggdb3'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    bincompat5005=undef
  Compiler:
    cc='afl-clang-fast'
    ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O0 -g -ggdb3'
    cppflags='-DDEBUGGING -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='4.2.1 Compatible Clang 3.9.1 (tags/RELEASE_391/rc2)'
    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/lib/llvm-3.9/bin/../lib/clang/3.9.1/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.24.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.24'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O0 -g -ggdb3 -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.25.9:
    lib
    /usr/local/lib/perl5/site_perl/5.25.9/x86_64-linux
    /usr/local/lib/perl5/site_perl/5.25.9
    /usr/local/lib/perl5/5.25.9/x86_64-linux
    /usr/local/lib/perl5/5.25.9


Environment for perl 5.25.9:
    HOME=/home/afl
    LANG=en_US.UTF-8
    LANGUAGE=en_US:en
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.22.1/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games
    PERLBREW_BASHRC_VERSION=0.78
    PERLBREW_HOME=/home/afl/.perlbrew
    PERLBREW_MANPATH=/home/afl/perlbrew/perls/perl-5.22.1/man
    PERLBREW_PATH=/home/afl/perlbrew/bin:/home/afl/perlbrew/perls/perl-5.22.1/bin
    PERLBREW_PERL=perl-5.22.1
    PERLBREW_ROOT=/home/afl/perlbrew
    PERLBREW_VERSION=0.78
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From @dur-randir

Forgot to attach crash example, here it is.

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From @dur-randir

0053

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From [Unknown Contact. See original ticket]

Forgot to attach crash example, here it is.

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From @khwilliamson

Thanks for finding this; fixed by
deb0ebcf5f07709222a2073bee27df845a1f14a1
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

@khwilliamson - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this as completed Jan 29, 2017
@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2017

From @iabyn

On Sun, Jan 29, 2017 at 03​:14​:25PM -0800, Karl Williamson via RT wrote​:

Thanks for finding this; fixed by
deb0ebcf5f07709222a2073bee27df845a1f14a1

I can't see that commit, but I do see this one​:

  commit d206794
  Author​: Karl Williamson <khw@​cpan.org>
  AuthorDate​: Sun Jan 29 15​:56​:20 2017 -0700
  Commit​: Karl Williamson <khw@​cpan.org>
  CommitDate​: Sun Jan 29 21​:05​:54 2017 -0700

  PATCH​: [perl #130666]​: Revert "toke.c, S_scan_ident()​: Don't take a "end of buffer" argument, use PL_bufend"
 
  This reverts commit b5248d1.

This appears to be cause of the following compiler warning​:

  toke.c​:9273​:41​: warning​: unused parameter 'send' [-Wunused-parameter]
  S_scan_ident(pTHX_ char *s, const char *send, char *dest, STRLEN destlen, I32 ck_uni)

because that commit seems have added back a 'send' parameter to
S_scan_ident(), but that function still uses PL_bufend for its actual
end-of-string delimiting.

So I'm not sure what the intent was, or how it should be fixed.

--
A problem shared is a problem doubled.

@p5pRT
Copy link
Author

p5pRT commented Feb 2, 2017

From @khwilliamson

On 02/01/2017 09​:25 AM, Dave Mitchell wrote​:

On Sun, Jan 29, 2017 at 03​:14​:25PM -0800, Karl Williamson via RT wrote​:

Thanks for finding this; fixed by
deb0ebcf5f07709222a2073bee27df845a1f14a1

I can't see that commit, but I do see this one​:

commit d2067945159644d284f8064efbd41024f9e8448a
Author&#8203;:     Karl Williamson \<khw@&#8203;cpan\.org>
AuthorDate&#8203;: Sun Jan 29 15&#8203;:56&#8203;:20 2017 \-0700
Commit&#8203;:     Karl Williamson \<khw@&#8203;cpan\.org>
CommitDate&#8203;: Sun Jan 29 21&#8203;:05&#8203;:54 2017 \-0700

PATCH&#8203;: \[perl \#130666\]&#8203;: Revert "toke\.c\, S\_scan\_ident\(\)&#8203;: Don't take a "end of buffer" argument\, use PL\_bufend"

This reverts commit b5248d1e210c2a723adae8e9b7f5d17076647431\.

This appears to be cause of the following compiler warning​:

toke\.c&#8203;:9273&#8203;:41&#8203;: warning&#8203;: unused parameter 'send' \[\-Wunused\-parameter\]
S\_scan\_ident\(pTHX\_ char \*s\, const char \*send\, char \*dest\, STRLEN destlen\, I32 ck\_uni\)

because that commit seems have added back a 'send' parameter to
S_scan_ident(), but that function still uses PL_bufend for its actual
end-of-string delimiting.

So I'm not sure what the intent was, or how it should be fixed.

Now fixed by this​:

  commit c9470cf
  Author​: Karl Williamson <khw@​cpan.org>
  Date​: Wed Feb 1 13​:15​:00 2017 -0700

toke.c​: Remove unused param from static function

Commit d206794 reverted commit
b5248d1. b5248 removed a parameter
from S_scan_ident, and changed its interior to use PL_bufend instead of
that parameter. The parameter had been used to limit how far into the
string being parsed scan_ident could look. In all calls to scan_ident
but one, the parameter was already PL_bufend. In the one call where it
wasn't, b5248 compensated by temporarily changing PL_bufend around the
call, running afoul, eventually, of the expectation that PL_bufend
points to a NUL.

I would have expected the reversion to add back both the parameter and
the uses of it, but apparently the function interior has changed enough
since the original commit, that it didn't even think there were
conflicts. As a result the parameter got added back, but not the uses
of it.

I tried both approaches to fix this​:
  1) to change the function to use the parameter;
  2) to simply delete the parameter.
Only the latter passed the test suite without error.

I then tried to understand why the parameter in the first place, and why
the kludge introduced by b5248 to work around removing it. It appears
to me that this is for the benefit of the intuit_more function to enable
it to discern $] from a $ ending a bracketed character class, by ending
the scan before the ']' when in a pattern.

The trouble is that modern scan_ident versions do not view themselves as
constrained by PL_bufend. If that is reached at a point where white
space is allowed, it will try appending the next input line and
continuing, thus changing PL_bufend. Thus the kludge in b5248 wouldn't
necessarily do the expected limiting anyway. The reason the approach
"1)" I tried didn't work was that the function continued to use the
original value, even after it had read in new things, instead of
accounting for those.

Hence approach "2)" is used. I'm a little nervous about this, as it may
lead to intuit_more() (which uses heuristics) having more cases where it
makes the wrong choice about $] vs [...$]. But I don't see a way around
this, and the pre-existing code could fail anyway.

Spotted by Dave Mitchell.

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

1 participant