Open p5pRT opened 7 years ago
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
9f141731d83a1ac6294a5580a5b11ff41490309a is the first bad commit commit 9f141731d83a1ac6294a5580a5b11ff41490309a 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=\
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
The RT System itself - Status changed from 'new' to 'open'
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:
b30fcab9d36ac78789465635e3b1009dcf6a9488 is the first bad commit commit b30fcab9d36ac78789465635e3b1009dcf6a9488 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*.
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.
The RT System itself - Status changed from 'new' to 'open'
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
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
@​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.
If I understand correctly\, this is bug in interpolation - and isn't a security issue.
Is that correct?
Tony
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
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
Migrated from rt.perl.org#130651 (status was 'open')
Searchable as RT130651$