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

regcomp.c:6881: REGEXP *Perl_re_op_compile(SV **const, int, OP *, const regexp_engine *, REGEXP *, _Bool *, U32, U32): Assertion `expr' failed #15841

Open
p5pRT opened this issue Jan 26, 2017 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 26, 2017

Migrated from rt.perl.org#130651 (status was 'open')

Searchable as RT130651$

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 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 program

qr!@​{return%0}0[(?{!

to cause an assertion failure on debugging builds and crash on regular
build. This is a regression between v5.16.0 and v5.18.0, bisect points
to

9f14173 is the first bad commit
commit 9f14173
Author​: David Mitchell <davem@​iabyn.com>
Date​: Fri Nov 4 10​:12​:20 2011 +0000

  Move bulk of pp_regcomp() into re_op_compile()

  When called, pp_regcomp() is presented with a list of SVs on the stack.
  Previously, it would perform (amongst other things)​:
  * overloading those SVs;
  * concatenating them;
  * detection of bare /$qr/;
  * detection of unchanged pattern;
  optionally followed by a call to the built-in or an external regexp
  compiler.

  Since we want to avoid premature concatenation (so that we can handle
  /$runtime(?{...})/), move all these activities from pp_regcomp() into
  re_op_compile().

  This makes re_op_compile() a bit cumbersome, with a large arg list,
  but I haven't found any way of only moving only a subset of the above.

  Note that a side-effect of this is that qr-overloading now works for all
  regex compilations, not just those reached via pp_regcomp(); in particular
  this now invokes the qr method rather than the "" method if available​:
  /(??{ $overloaded_object })/

GDB info about the crash location​:

(gdb) bt
#0 __GI_raise (sig=sig@​entry=6) at ../sysdeps/unix/sysv/linux/raise.c​:58
#1 0x00007f638067840a in __GI_abort () at abort.c​:89
#2 0x00007f638066fe47 in __assert_fail_base (fmt=<optimized out>,
assertion=assertion@​entry=0x7f6381d5c686 "expr",
  file=file@​entry=0x7f6381d5a198 "regcomp.c", line=line@​entry=6881,
  function=function@​entry=0x7f6381d63580 <__PRETTY_FUNCTION__.16856>
"Perl_re_op_compile") at assert.c​:92
#3 0x00007f638066fef2 in __GI___assert_fail (assertion=0x7f6381d5c686
"expr", file=0x7f6381d5a198 "regcomp.c", line=6881,
  function=0x7f6381d63580 <__PRETTY_FUNCTION__.16856>
"Perl_re_op_compile") at assert.c​:101
#4 0x00007f6381aa179b in Perl_re_op_compile (patternp=0x7f63838a0b58,
pat_count=0, expr=0x0, eng=0x7f6381fe6540 <PL_core_reg_engine>,
old_re=0x0,
  is_bare_re=0x7ffeaf6f0a5a, orig_rx_flags=0, pm_flags=1342177280)
at regcomp.c​:6881
#5 0x00007f6381c080f1 in Perl_pp_regcomp () at pp_ctl.c​:107
#6 0x00007f6381afb6e9 in Perl_runops_debug () at dump.c​:2444
#7 0x00007f63819f4936 in S_run_body (oldscope=1) at perl.c​:2528
#8 0x00007f63819f3eb4 in perl_run (my_perl=0x7f638389c010) at perl.c​:2451
#9 0x00007f63819aeeee in main (argc=2, argv=0x7ffeaf6f0e28,
env=0x7ffeaf6f0e40) at perlmain.c​:123

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 27, 2017

From @iabyn

On Thu, Jan 26, 2017 at 07​:13​:37AM -0800, Sergey Aleynikov wrote​:

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

qr!@​{return%0}0[(?{!

A bit of an edge case this. runtime qr//s which include code blocks
are wrapped in a hidden sub to make all the closurey stuff work correctly.
So for example

  $r = qr/a$b(?{...})/

is roughly equivalent to

  @​args = sub {
  ('a', $b, sub{...});
  }->();
  $args = join '', @​args;
  $r = qr/$args/

(except that the inner sub isn't actually a separate anon sub - its just a
code block compiled as part of the outer anon sub, and the code block gets
passed to the regex compiler as-is rather than being stringified and
concatenated.)

The 'return' inside the @​{...} expression is causing a return from the
hidden sub, returning nothing, which then causes an assert fail inside
Perl_re_op_compile, since its expecting at least 1 arg.

The most immediate fix would be to make Perl_re_op_compile() robust in the
presence of no (or unexpected) args, but it is semantically incorrect.
The return should cause a return out of two levels of sub - the hidden
one, and the expected one which the qr// is enclosed in. There's already
a mechanism for returning from two nested subs​: CXp_SUB_RE_FAKE; but
I don't yet see an easy way to get it set in this case, without
polluting pp_entersub with more flags and special cases.

I need to think some more.

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Feb 2, 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 program

qr!0(?{})${return''}!

to use an initialized memory slot as a pointer to data. This is a
regression between v5.16 and v5.18, bisect points to​:

b30fcab is the first bad commit
commit b30fcab
Author​: David Mitchell <davem@​iabyn.com>
Date​: Wed Nov 30 13​:40​:15 2011 +0000

  preserve code blocks in interpolated qr//s

  This now works​:

  { my $x = 1; $r = qr/(??{$x})/ }
  my $x = 2;
  print "ok\n" if "1" =~ /^$r$/;

  When a qr// is interpolated into another pattern, the pattern is still
  recompiled using the stringified qr, but now the pre-compiled code blocks
  from the qr are reused rather than being re-compiled, so it behaves like a
  closure.

GDB info about the program state is​:

#0 0x00007f755ebcefb8 in S_SvREFCNT_dec (sv=0xcccccccccccccccc) at inline.h​:185
185 U32 rc = SvREFCNT(sv);
(gdb) bt
#0 0x00007f755ebcefb8 in S_SvREFCNT_dec (sv=0xcccccccccccccccc) at inline.h​:185
#1 0x00007f755ebec7a6 in S_free_codeblocks (cbs=0x7f755d207ff0) at
regcomp.c​:6141
#2 0x00007f755ec32e38 in Perl_regfree_internal (rx=0x7f755d4fe658) at
regcomp.c​:19539
#3 0x00007f755ec31ea5 in Perl_pregfree2 (rx=0x7f755d4fe658) at regcomp.c​:19393
#4 0x00007f755ecedbda in Perl_sv_clear (orig_sv=0x7f755d4fe658) at sv.c​:6609
#5 0x00007f755ecf0d8d in Perl_sv_free2 (sv=0x7f755d4fe658, rc=1) at sv.c​:7063
#6 0x00007f755eafef0d in S_SvREFCNT_dec (sv=0x7f755d4fe658) at inline.h​:189
#7 0x00007f755eb019e9 in Perl_op_clear (o=0x7f755d23ff98) at op.c​:1056
#8 0x00007f755eb014f0 in Perl_op_free (o=0x7f755d23ff98) at op.c​:855
#9 0x00007f755eb3ee1f in perl_destruct (my_perl=0x7f755f12cfff) at perl.c​:832
#10 0x00007f755eafdf87 in main (argc=3, argv=0x7fff346cf748,
env=0x7fff346cf768) at perlmain.c​:134

Pattern 0xcccccccccccccccc is an afl's pattern to poison malloc'ed
memory. If not for the SIGSEGV here, S_SvREFCNT_dec would perform a
write into sv_refcnt of what it thinks is an SV*.

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 Feb 3, 2017

From @iabyn

On Thu, Feb 02, 2017 at 07​:58​:44AM -0800, Sergey Aleynikov wrote​:

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

This is a bug report for perl from sergey.aleynikov@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.25.9.

-----------------------------------------------------------------
[Please describe your issue here]

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

qr!0(?{})${return''}!

to use an initialized memory slot as a pointer to data. This is a
regression between v5.16 and v5.18, bisect points to​:

This is a variant on the issue discussed in RT #130651

--
I before E. Except when it isn't.

@p5pRT
Copy link
Author

p5pRT commented Feb 3, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Jul 6, 2017

From @tonycoz

On Fri, 03 Feb 2017 01​:27​:17 -0800, davem wrote​:

On Thu, Feb 02, 2017 at 07​:58​:44AM -0800, Sergey Aleynikov wrote​:

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

qr!0(?{})${return''}!

to use an initialized memory slot as a pointer to data. This is a
regression between v5.16 and v5.18, bisect points to​:

This is a variant on the issue discussed in RT #130651

If I understand #130651, then this isn't a seecurity issue, since it would require either the attacker to be able to feed code directly to the interpreter, or for C<use re 'eval';> to be enabled.

Am I wrong?

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @tonycoz

On Fri, 27 Jan 2017 07​:05​:43 -0800, davem wrote​:

On Thu, Jan 26, 2017 at 07​:13​:37AM -0800, Sergey Aleynikov wrote​:

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

qr!@​{return%0}0[(?{!

A bit of an edge case this. runtime qr//s which include code blocks
are wrapped in a hidden sub to make all the closurey stuff work correctly.
So for example

$r = qr/a$b\(?\{\.\.\.\}\)/

is roughly equivalent to

@&#8203;args = sub \{
            \('a'\, $b\, sub\{\.\.\.\}\);
        \}\->\(\);
$args = join ''\, @&#8203;args;
$r = qr/$args/

(except that the inner sub isn't actually a separate anon sub - its just a
code block compiled as part of the outer anon sub, and the code block gets
passed to the regex compiler as-is rather than being stringified and
concatenated.)

The 'return' inside the @​{...} expression is causing a return from the
hidden sub, returning nothing, which then causes an assert fail inside
Perl_re_op_compile, since its expecting at least 1 arg.

The most immediate fix would be to make Perl_re_op_compile() robust in the
presence of no (or unexpected) args, but it is semantically incorrect.
The return should cause a return out of two levels of sub - the hidden
one, and the expected one which the qr// is enclosed in. There's already
a mechanism for returning from two nested subs​: CXp_SUB_RE_FAKE; but
I don't yet see an easy way to get it set in this case, without
polluting pp_entersub with more flags and special cases.

I need to think some more.

If I understand correctly, this is bug in interpolation - and isn't a security issue.

Is that correct?

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @tonycoz

On Tue, 22 Aug 2017 22​:46​:34 -0700, tonyc wrote​:

If I understand correctly, this is bug in interpolation - and isn't a
security issue.

Is that correct?

Nevermind, wrong ticket.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2018

From @tonycoz

On Fri, 03 Feb 2017 01​:27​:17 -0800, davem wrote​:

On Thu, Feb 02, 2017 at 07​:58​:44AM -0800, Sergey Aleynikov wrote​:

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

This is a bug report for perl from sergey.aleynikov@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.25.9.

-----------------------------------------------------------------
[Please describe your issue here]

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

qr!0(?{})${return''}!

to use an initialized memory slot as a pointer to data. This is a
regression between v5.16 and v5.18, bisect points to​:

This is a variant on the issue discussed in RT #130651

Now public, and I'll merge it into 130651.

Tony

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

2 participants