Skip Menu |
Report information
Id: 129881
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: brian.carpenter [at] gmail.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: (no value)
Type: (no value)
Perl Version: (no value)
Fixed In: (no value)



Subject: heap-buffer-overflow Perl_pad_sv (pad.c:1354)
To: perl5-security-report [...] perl.org
From: "Brian 'geeknik' Carpenter" <brian.carpenter [...] gmail.com>
Date: Fri, 14 Oct 2016 16:51:33 -0500
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.
Download test145.gz
application/x-gzip 541b

Message body not shown because it is not plain text.

RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 320b
On Fri, 14 Oct 2016 14:52:33 -0700, brian.carpenter@gmail.com wrote: Show quoted text
> AFL+ASAN and Perl v5.25.6 (v5.25.5-104-gaff2be5):
The attached produces the same error for me. Show quoted text
> 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
Subject: 129881b.pl.gz
Download 129881b.pl.gz
application/gzip 340b

Message body not shown because it is not plain text.

Date: Sun, 6 Nov 2016 23:33:25 -0600
To: perl5-security-report [...] perl.org
Subject: Re: [perl #129881] heap-buffer-overflow Perl_pad_sv (pad.c:1354)
From: "Brian 'geeknik' Carpenter" <brian.carpenter [...] gmail.com>
Most likely.
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
On Sun, 06 Nov 2016 21:12:54 -0800, tonyc wrote: Show quoted text
> 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;
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 928b
On Mon, 07 Nov 2016 02:12:39 -0800, hv wrote: Show quoted text
> 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
From: demerphq <demerphq [...] gmail.com>
Date: Mon, 7 Nov 2016 12:03:34 +0100
To: "perl5-security-report [...] perl.org" <perl5-security-report [...] perl.org>
Subject: Re: [perl #129881] heap-buffer-overflow Perl_pad_sv (pad.c:1354)
CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.2k
On 7 November 2016 at 11:58, Hugo van der Sanden via RT <perl5-security-report@perl.org> wrote: Show quoted text
> 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/"
Subject: Re: [perl #129881] heap-buffer-overflow Perl_pad_sv (pad.c:1354)
From: Dave Mitchell <davem [...] iabyn.com>
To: demerphq <demerphq [...] gmail.com>
CC: "perl5-security-report [...] perl.org" <perl5-security-report [...] perl.org>, rt-deliver-to-perl5-security-report [...] rt.perl.org
Date: Tue, 14 Feb 2017 18:11:30 +0000
On Mon, Nov 07, 2016 at 12:03:34PM +0100, demerphq wrote: Show quoted text
> 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 4b9c7caeaecf4e9df0be3a2e296644f763f775d6 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.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org