heap-buffer-overflow Perl_pad_sv (pad.c:1354)

[p5pRT]

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

Searchable as RT129881$

[p5pRT]

From @geeknik

AFL+ASAN and Perl v5.25.6 (v5.25.5-104-gaff2be5)​:

==29437==ERROR​: AddressSanitizer​: heap-buffer-overflow on address
0x60300000ea58 at pc 0x0000006c7e53 bp 0x7fffc6f53fb0 sp 0x7fffc6f53fa8
READ of size 8 at 0x60300000ea58 thread T0
  #0 0x6c7e52 in Perl_pad_sv /root/perl/pad.c​:1354​:5
  #1 0x9c240c in Perl_pp_subtract /root/perl/pp.c​:1893​:10
  #2 0x7f44b3 in Perl_runops_debug /root/perl/dump.c​:2246​:23
  #3 0x5a12b6 in S_run_body /root/perl/perl.c​:2526​:2
  #4 0x5a12b6 in perl_run /root/perl/perl.c​:2449
  #5 0x4de60d in main /root/perl/perlmain.c​:123​:9
  #6 0x7fa6f1079b44 in __libc_start_main
/build/glibc-daoqzt/glibc-2.19/csu/libc-start.c​:287
  #7 0x4de27c in _start (/root/perl/perl+0x4de27c)

0x60300000ea5c is located 0 bytes to the right of 28-byte region
[0x60300000ea40,0x60300000ea5c)
allocated by thread T0 here​:
  #0 0x4c0bfb in malloc (/root/perl/perl+0x4c0bfb)
  #1 0x7f82f7 in Perl_safesysmalloc /root/perl/util.c​:153​:21

SUMMARY​: AddressSanitizer​: heap-buffer-overflow /root/perl/pad.c​:1354
Perl_pad_sv
Shadow bytes around the buggy address​:
  0x0c067fff9cf0​: fd fd fa fa fd fd fd fd fa fa fd fd fd fd fa fa
  0x0c067fff9d00​: fd fd fd fd fa fa fd fd fd fd fa fa fd fd fd fd
  0x0c067fff9d10​: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd
  0x0c067fff9d20​: fd fa fa fa fd fd fd fd fa fa 00 00 00 02 fa fa
  0x0c067fff9d30​: 00 00 00 03 fa fa 00 00 00 00 fa fa 00 00 00 fa
=>0x0c067fff9d40​: fa fa fd fd fd fa fa fa 00 00 00[04]fa fa 00 00
  0x0c067fff9d50​: 00 fa fa fa 00 00 05 fa fa fa 00 00 00 06 fa fa
  0x0c067fff9d60​: 00 00 00 00 fa fa 00 00 00 00 fa fa 00 00 00 00
  0x0c067fff9d70​: fa fa fd fd fd fd fa fa 00 00 00 00 fa fa fd fd
  0x0c067fff9d80​: fd fd fa fa 00 00 00 00 fa fa fd fd fd fd fa fa
  0x0c067fff9d90​: 00 00 00 00 fa fa 00 00 00 00 fa fa fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes)​:
  Addressable​: 00
  Partially addressable​: 01 02 03 04 05 06 07
  Heap left redzone​: fa
  Heap right redzone​: fb
  Freed heap region​: fd
  Stack left redzone​: f1
  Stack mid redzone​: f2
  Stack right redzone​: f3
  Stack partial redzone​: f4
  Stack after return​: f5
  Stack use after scope​: f8
  Global redzone​: f9
  Global init order​: f6
  Poisoned by user​: f7
  Container overflow​: fc
  ASan internal​: fe
==29437==ABORTING

Valgrind and non-ASAN Perl v5.25.6 (v5.25.5-104-gaff2be5)​:

==21722== Invalid read of size 8
==21722== at 0x48D748​: Perl_pad_sv (pad.c​:1354)
==21722== by 0x54BCB0​: Perl_pp_subtract (pp.c​:1893)
==21722== by 0x4D71A1​: Perl_runops_debug (dump.c​:2246)
==21722== by 0x453156​: S_run_body (perl.c​:2526)
==21722== by 0x453156​: perl_run (perl.c​:2449)
==21722== by 0x421984​: main (perlmain.c​:123)
==21722== Address 0x5f807a8 is 24 bytes after a block of size 48 in arena
"client"
==21722==
==21722== Invalid read of size 4
==21722== at 0x54B98B​: Perl_pp_subtract (pp.c​:1918)
==21722== by 0x4D71A1​: Perl_runops_debug (dump.c​:2246)
==21722== by 0x453156​: S_run_body (perl.c​:2526)
==21722== by 0x453156​: perl_run (perl.c​:2449)
==21722== by 0x421984​: main (perlmain.c​:123)
==21722== Address 0x7c is not stack'd, malloc'd or (recently) free'd
==21722==
==21722==
==21722== Process terminating with default action of signal 11 (SIGSEGV)
==21722== Access not within mapped region at address 0x7C
==21722== at 0x54B98B​: Perl_pp_subtract (pp.c​:1918)
==21722== by 0x4D71A1​: Perl_runops_debug (dump.c​:2246)
==21722== by 0x453156​: S_run_body (perl.c​:2526)
==21722== by 0x453156​: perl_run (perl.c​:2449)
==21722== by 0x421984​: main (perlmain.c​:123)
==21722== If you believe this happened as a result of a stack
==21722== overflow in your program's main thread (unlikely but
==21722== possible), you can try to increase the size of the
==21722== main thread stack using the --main-stacksize= flag.
==21722== The main thread stack size used in this run was 8388608.
Segmentation fault

afl-tmin minimized the crash into an assertion failure, so I didn't
attached the minimized test case.

[p5pRT]

From @geeknik

test145.gz

[p5pRT]

From @tonycoz

On Fri, 14 Oct 2016 14​:52​:33 -0700, brian.carpenter@​gmail.com wrote​:

AFL+ASAN and Perl v5.25.6 (v5.25.5-104-gaff2be5)​:

The attached produces the same error for me.

afl-tmin minimized the crash into an assertion failure, so I didn't
attached the minimized test case.

Did you open that as a different issue?

Tony

[p5pRT]

From @tonycoz

129881b.pl.gz

[p5pRT]

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

[p5pRT]

From @geeknik

Most likely.

[p5pRT]

From @hvds

On Sun, 06 Nov 2016 21​:12​:54 -0800, tonyc wrote​:

On Fri, 14 Oct 2016 14​:52​:33 -0700, brian.carpenter@​gmail.com wrote​:

AFL+ASAN and Perl v5.25.6 (v5.25.5-104-gaff2be5)​:

The attached produces the same error for me.

I was able to reduce and simplify it some, as below.

When I run this, I get a SEGV in sv_setpvn called from pp_concat for "state = $state\n" with TARG clearly corrupt (I've seen values such as 0x31, 0x01 and 0x60 while reducing).

The useless '+' on '(?{ local $d = 1 })' is required.

With valgrind, it instead gives the same 'Invalid read of size 8' at pad_sv (pad.c​:1354) called from pp_concat (pp_hot.c​:258), which is presumably the dATARGET getting the corrupt pointer - the original case had it calling from pp_subtract, which I think was from the '(...)[$q - 6]', and quite plausibly the first reference to a TARG in the subst replacement eval of the original version.

Hugo

---

use warnings;

$z = qr{
  (?{ local $d = 1 })+
  X (?{ $d++ })
}x;

$_ = "A";
1 while s{
  A ($z) B
|
  B (?{ chr 120 - $y })
|
  (?{ 1 })
}{
  print "q​: $q\n";
  "x"
}ex;

[p5pRT]

From @hvds

On Mon, 07 Nov 2016 02​:12​:39 -0800, hv wrote​:

I was able to reduce and simplify it some, as below.

I don't know much about pads, but running with -DX shows​:

% ./perl -DX testprog
Pad 0x26e7358[0x26f5f40] new​: compcv=0x26e7340 name=0x26f5f70 flags=0x0
Pad 0x2706ae0[0x270bad0] new​: compcv=0x2706ac8 name=0x270bb20 flags=0x6
Pad 0x2706ae0[0x270bad0] alloc​: 1 for postinc
Pad 0x26e7358[0x26f5f40] alloc​: 1 for qr
Pad 0x26e7358[0x26f5f40] alloc​: 2 for subtract
Pad 0x26e7358[0x26f5f40] alloc​: 3 for chr
Pad 0x26e7358[0x26f5f40] alloc​: 4 for concat
Pad 0x26e7358[0x270bcb0] alloc​: 5 for concat
Pad 0x26e7358[0x270bd30] alloc​: 6 for stringify
Pad 0x26e7358[0x270bd30] free​: 6

EXECUTING...

Pad 0x2706ae0[0x270bad0] sv​: 4 sv=0x0
perl​: sv.c​:4918​: Perl_sv_setpvn​: Assertion `sv' failed.
Aborted (core dumped)
%

.. which appears to show we're wrongly looking at the pad from the (?{ $d++ }).

Hugo

[p5pRT]

From @demerphq

On 7 November 2016 at 11​:58, Hugo van der Sanden via RT
<perl5-security-report@​perl.org> wrote​:

On Mon, 07 Nov 2016 02​:12​:39 -0800, hv wrote​:

I was able to reduce and simplify it some, as below.

I don't know much about pads, but running with -DX shows​:

% ./perl -DX testprog
Pad 0x26e7358[0x26f5f40] new​: compcv=0x26e7340 name=0x26f5f70 flags=0x0
Pad 0x2706ae0[0x270bad0] new​: compcv=0x2706ac8 name=0x270bb20 flags=0x6
Pad 0x2706ae0[0x270bad0] alloc​: 1 for postinc
Pad 0x26e7358[0x26f5f40] alloc​: 1 for qr
Pad 0x26e7358[0x26f5f40] alloc​: 2 for subtract
Pad 0x26e7358[0x26f5f40] alloc​: 3 for chr
Pad 0x26e7358[0x26f5f40] alloc​: 4 for concat
Pad 0x26e7358[0x270bcb0] alloc​: 5 for concat
Pad 0x26e7358[0x270bd30] alloc​: 6 for stringify
Pad 0x26e7358[0x270bd30] free​: 6

EXECUTING...

Pad 0x2706ae0[0x270bad0] sv​: 4 sv=0x0
perl​: sv.c​:4918​: Perl_sv_setpvn​: Assertion `sv' failed.
Aborted (core dumped)
%

.. which appears to show we're wrongly looking at the pad from the (?{ $d++ }).

If I remove the print i get a different assert fail (but i am on
hacked branch, so it may not be relevant).

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

[p5pRT]

From @iabyn

On Mon, Nov 07, 2016 at 12​:03​:34PM +0100, demerphq wrote​:

On 7 November 2016 at 11​:58, Hugo van der Sanden via RT
<perl5-security-report@​perl.org> wrote​:

On Mon, 07 Nov 2016 02​:12​:39 -0800, hv wrote​:

I was able to reduce and simplify it some, as below.

I don't know much about pads, but running with -DX shows​:

% ./perl -DX testprog
Pad 0x26e7358[0x26f5f40] new​: compcv=0x26e7340 name=0x26f5f70 flags=0x0
Pad 0x2706ae0[0x270bad0] new​: compcv=0x2706ac8 name=0x270bb20 flags=0x6
Pad 0x2706ae0[0x270bad0] alloc​: 1 for postinc
Pad 0x26e7358[0x26f5f40] alloc​: 1 for qr
Pad 0x26e7358[0x26f5f40] alloc​: 2 for subtract
Pad 0x26e7358[0x26f5f40] alloc​: 3 for chr
Pad 0x26e7358[0x26f5f40] alloc​: 4 for concat
Pad 0x26e7358[0x270bcb0] alloc​: 5 for concat
Pad 0x26e7358[0x270bd30] alloc​: 6 for stringify
Pad 0x26e7358[0x270bd30] free​: 6

EXECUTING...

Pad 0x2706ae0[0x270bad0] sv​: 4 sv=0x0
perl​: sv.c​:4918​: Perl_sv_setpvn​: Assertion `sv' failed.
Aborted (core dumped)
%

.. which appears to show we're wrongly looking at the pad from the (?{ $d++ }).

If I remove the print i get a different assert fail (but i am on
hacked branch, so it may not be relevant).

Fixed by the following commit, which I've just pushed as part of a larger
merge of commits related to re_evals, WHILEM and scope.

The specific issue with this ticket was that under certain circumstances,
a pattern which executes multiple code bocks, where those code blocks come
from multiple CVs (e.g. via an embedded qr//), could leave PL_comppad
pointing at the wrong pad, and badness like the one shown in this ticket
ensues. The specific issue was introduced (I think) by my 5.24.0 context
work. Similar issues (and much worse) used to happen before 5.18.0 when
I completely reworked the re-eval implementation.

I don't think its a realistic security issue. It requires executing a
pattern containing exactly the right sets of embedded (?{...}) code blocks
from multiple sources, against a particular string. If the attacker can
control the code blocks then you've already lost; if they don't, then they
would be extremely lucky to come across a pre-existing exploitable pattern
(hence why this issue has only been detected via fuzzing rather than via
any live code).

commit 4b9c7ca
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Sat Feb 11 11​:53​:41 2017 +0000
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue Feb 14 17​:49​:58 2017 +0000

  fix pad/scope issue in re_evals
 
  RT #129881 heap-buffer-overflow Perl_pad_sv
 
  In some circumstances involving a pattern which has embedded code blocks
  from more than one source, e.g.
 
  my $r = qr{(?{1;}){2}X};
  "" =~ /$r|(?{1;})/;
 
  the wrong PL_comppad could be active while doing a LEAVE_SCOPE() or on
  exit from the pattern.
 
  This was mainly due to the big context stack changes in 5.24.0 - in
  particular, since POP_MULTICALL() now does CX_LEAVE_SCOPE(cx) *before*
  restoring PL_comppad, the (correct) unwinding of any SAVECOMPPAD's was
  being followed by C<PL_comppad = cx->blk_sub.prevcomppad>, which wasn't
  necessarily a sensible value.
 
  To fix this, record the value of PL_savestack_ix at entry to S_regmatch(),
  and set the cx->blk_oldsaveix of the MULTICALL to this value when pushed.
  On exit from S_regmatch, we either POP_MULTICALL which will do a
  LEAVE_SCOPE(cx->blk_oldsaveix), or in the absense of any EVAL, do the
  explicit but equivalent LEAVE_SCOPE(orig_savestack_ix).
 
  Note that this is a change in behaviour to S_regmatch() - formerly it
  wouldn't necessarily clear the savestack completely back the point of
  entry - that would get left to do by its caller, S_regtry(), or indirectly
  by Perl_regexec_flags(). This shouldn't make any practical difference, but
  is tidier and less likely to introduce bugs later.

--
Dave's first rule of Opera​:
If something needs saying, say it​: don't warble it.

[p5pRT]

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