Skip Menu |
Report information
Id: 129012
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)



From: "Brian 'geeknik' Carpenter" <brian.carpenter [...] gmail.com>
Date: Sat, 20 Aug 2016 02:23:35 -0500
To: perl5-security-report [...] perl.org
Subject: heap-buffer-overflow Perl_fbm_instr (util.c:974)
Download (untitled) / with headers
text/plain 2.6k
The attached test case triggers a heap-buffer-overflow in Perl_fbm_instr (util.c:974). This was found with AFL, ASAN and libdislocator.so affects v5.25.4 (v5.25.3-305-g8c6b0c7). Perl 5.20.2 exits with no output, errors, etc.

==21934==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000e05b at pc 0x0000007fb01f bp 0x7fff655c7a10 sp 0x7fff655c7a08
READ of size 1 at 0x60200000e05b thread T0
    #0 0x7fb01e in Perl_fbm_instr /root/perl/util.c:974:10
    #1 0xb4859c in Perl_re_intuit_start /root/perl/regexec.c:1063:21
    #2 0xb3a397 in Perl_regexec_flags /root/perl/regexec.c:2986:6
    #3 0x8cb155 in Perl_pp_subst /root/perl/pp_hot.c:2999:10
    #4 0x7f1d93 in Perl_runops_debug /root/perl/dump.c:2234:23
    #5 0x5a11d6 in S_run_body /root/perl/perl.c:2524:2
    #6 0x5a11d6 in perl_run /root/perl/perl.c:2447
    #7 0x4de85d in main /root/perl/perlmain.c:123:9
    #8 0x7f60836abb44 in __libc_start_main /build/glibc-uPj9cH/glibc-2.19/csu/libc-start.c:287
    #9 0x4de4cc in _start (/root/perl/perl+0x4de4cc)

0x60200000e05b is located 0 bytes to the right of 11-byte region [0x60200000e050,0x60200000e05b)
allocated by thread T0 here:
    #0 0x4c0e4b in malloc (/root/perl/perl+0x4c0e4b)
    #1 0x7f5bd7 in Perl_safesysmalloc /root/perl/util.c:153:21

SUMMARY: AddressSanitizer: heap-buffer-overflow /root/perl/util.c:974 Perl_fbm_instr
Shadow bytes around the buggy address:
  0x0c047fff9bb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9bc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9bd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9be0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9bf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff9c00: fa fa 00 02 fa fa 00 02 fa fa 00[03]fa fa fd fd
  0x0c047fff9c10: fa fa 00 02 fa fa 00 02 fa fa fd fa fa fa 00 02
  0x0c047fff9c20: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c047fff9c30: fa fa fd fd fa fa fd fd fa fa 00 02 fa fa fd fd
  0x0c047fff9c40: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c047fff9c50: fa fa 00 02 fa fa fd fd fa fa fd fd fa fa 02 fa
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
==21934==ABORTING

Download test303.gz
application/x-gzip 56b

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 2.5k
On Sat Aug 20 00:24:38 2016, brian.carpenter@gmail.com wrote: Show quoted text
> The attached test case triggers a heap-buffer-overflow in Perl_fbm_instr > (util.c:974). This was found with AFL, ASAN and libdislocator.so > affects v5.25.4 (v5.25.3-305-g8c6b0c7). Perl 5.20.2 exits with no output, > errors, etc.
valgrind finds errors as far back as 5.12.0 (which is as far I cared to look.) From valgrind this looks like: ==19661== Invalid read of size 1 ==19661== at 0x54161C: Perl_fbm_instr (util.c:973) ==19661== by 0x6AB2B7: Perl_re_intuit_start (regexec.c:1063) ==19661== by 0x6B5A42: Perl_regexec_flags (regexec.c:2986) ==19661== by 0x59BE4B: Perl_pp_subst (pp_hot.c:2999) ==19661== by 0x53DE46: Perl_runops_debug (dump.c:2234) ==19661== by 0x447DDE: S_run_body (perl.c:2525) ==19661== by 0x447409: perl_run (perl.c:2448) ==19661== by 0x7239B4: main (miniperlmain.c:129) ==19661== Address 0x5f83dab is 0 bytes after a block of size 11 alloc'd ==19661== at 0x4C28C20: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==19661== by 0x53F629: Perl_safesysmalloc (util.c:153) ==19661== by 0x5AA3A6: Perl_sv_grow (sv.c:1605) ==19661== by 0x5CA9DA: Perl_sv_setsv_flags (sv.c:4727) ==19661== by 0x589BBE: Perl_pp_sassign (pp_hot.c:226) ==19661== by 0x53DE46: Perl_runops_debug (dump.c:2234) ==19661== by 0x447DDE: S_run_body (perl.c:2525) ==19661== by 0x447409: perl_run (perl.c:2448) ==19661== by 0x7239B4: main (miniperlmain.c:129) Unfortunately vgdb misbehaves and in gdb produces a backtrace: 0x000000000054161c in Perl_save_re_context () at regcomp.c:19828 19828 = (GV**)hv_fetch(PL_defstash, digits, len, 0); (gdb) bt #0 0x000000000054161c in Perl_save_re_context () at regcomp.c:19828 #1 0x00000000006ab2b8 in S_do_trans_complex_utf8 (sv=0x5f83dbc) at doop.c:615 #2 0x00000000006b5a43 in Perl_nextargv (gv=0x78, nomagicopen=false) at doio.c:882 #3 0x000000000059be4c in S_unshare_hek_or_pvn (hek=0x5f59150, str=0xffffffffff000690 <error: Cannot access memory at address 0xffffffffff000690>, len=0, hash=0) at hv.c:2871 #4 0x000000000053de47 in Perl_regdump (r=0x1) at regcomp.c:18775 #5 0x0000000000447ddf in Perl_ck_bitop (o=0x5f54040) at op.c:9502 #6 0x000000000044740a in Perl_newFORM (floor=0, o=0xa801c0, block=0x0) at op.c:9234 #7 0x00000000007239b5 in S_next_symbol (symptr=0x5f55e28) at pp_pack.c:701 #8 0x0000000005bcab45 in ?? () #9 0x0000000000000000 in ?? () It looks like the second time fbm_instr() is called an out of range value for bigend is supplied. Otherwise I haven't worked out what's going on. Tony
Subject: Re: [perl #129012] heap-buffer-overflow Perl_fbm_instr (util.c:974)
To: Tony Cook via RT <perl5-security-report [...] perl.org>
Date: Tue, 23 Aug 2016 08:57:40 +0100
From: Dave Mitchell <davem [...] iabyn.com>
CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 338b
On Mon, Aug 22, 2016 at 11:08:00PM -0700, Tony Cook via RT wrote: Show quoted text
> It looks like the second time fbm_instr() is called an out of range > value for bigend is supplied. Otherwise I haven't worked out what's > going on.
This is an area of code that I've worked on before, so I'll have a look. -- My get-up-and-go just got up and went.
From: Dave Mitchell <davem [...] iabyn.com>
CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Date: Wed, 24 Aug 2016 14:10:29 +0100
To: Tony Cook via RT <perl5-security-report [...] perl.org>
Subject: Re: [perl #129012] heap-buffer-overflow Perl_fbm_instr (util.c:974)
Download (untitled) / with headers
text/plain 1.8k
On Tue, Aug 23, 2016 at 08:57:40AM +0100, Dave Mitchell wrote: Show quoted text
> On Mon, Aug 22, 2016 at 11:08:00PM -0700, Tony Cook via RT wrote:
> > It looks like the second time fbm_instr() is called an out of range > > value for bigend is supplied. Otherwise I haven't worked out what's > > going on.
> > This is an area of code that I've worked on before, so I'll have a look.
Now fixed with the following. Turns out its not a security concern. commit 71a9d1055562b01938400494965dac70b3a685c5 Author: David Mitchell <davem@iabyn.com> AuthorDate: Wed Aug 24 13:21:04 2016 +0100 Commit: David Mitchell <davem@iabyn.com> CommitDate: Wed Aug 24 13:30:33 2016 +0100 re_untuit_start() avoid overshoot with utf8 RT #129012 re_untuit_start() is run before doing a "proper" regex match, to either quickly reject a match or to find the earliest position in a string where the match could occur. Part of its action is to search within the string for a known substring which forms a part of the pattern. If that substring is utf8, with multiple bytes per character, then the calculation of the highest point in the string where its worth searching for the substring, could overshoot the end of the string. It's a (mostly) harmless issue, since apart from the issue of reading a few bytes beyond the end of a string (which might cause a problem if the string is memory mapped for example), the only concern is that in theory (although extremely unlikely) a spurious match for a substring could be found partly beyond the end of the string, resulting in the full RE engine being called to (correctly) do the match, when otherwise the match could have been more quickly rejected. -- The Enterprise successfully ferries an alien VIP from one place to another without serious incident. -- Things That Never Happen in "Star Trek" #7
Subject: Re: [perl #129012] heap-buffer-overflow Perl_fbm_instr (util.c:974)
To: Dave Mitchell <davem [...] iabyn.com>
Date: Wed, 24 Aug 2016 15:31:11 +0200
From: demerphq <demerphq [...] gmail.com>
CC: Tony Cook via RT <perl5-security-report [...] perl.org>, rt-deliver-to-perl5-security-report [...] rt.perl.org
Shouldnt that be "intuit" and not "untuit" ? On 24 August 2016 at 15:10, Dave Mitchell <davem@iabyn.com> wrote: Show quoted text
> On Tue, Aug 23, 2016 at 08:57:40AM +0100, Dave Mitchell wrote:
>> On Mon, Aug 22, 2016 at 11:08:00PM -0700, Tony Cook via RT wrote:
>> > It looks like the second time fbm_instr() is called an out of range >> > value for bigend is supplied. Otherwise I haven't worked out what's >> > going on.
>> >> This is an area of code that I've worked on before, so I'll have a look.
> > Now fixed with the following. Turns out its not a security concern. > > commit 71a9d1055562b01938400494965dac70b3a685c5 > Author: David Mitchell <davem@iabyn.com> > AuthorDate: Wed Aug 24 13:21:04 2016 +0100 > Commit: David Mitchell <davem@iabyn.com> > CommitDate: Wed Aug 24 13:30:33 2016 +0100 > > re_untuit_start() avoid overshoot with utf8 > > RT #129012 > > re_untuit_start() is run before doing a "proper" regex match, to either > quickly reject a match or to find the earliest position in a string where > the match could occur. Part of its action is to search within the string > for a known substring which forms a part of the pattern. > > If that substring is utf8, with multiple bytes per character, then > the calculation of the highest point in the string where its worth > searching for the substring, could overshoot the end of the string. > > It's a (mostly) harmless issue, since apart from the issue of reading a > few bytes beyond the end of a string (which might cause a problem if the > string is memory mapped for example), the only concern is that in theory > (although extremely unlikely) a spurious match for a substring could be > found partly beyond the end of the string, resulting in the full RE engine > being called to (correctly) do the match, when otherwise the match could > have been more quickly rejected. > > > -- > The Enterprise successfully ferries an alien VIP from one place to another > without serious incident. > -- Things That Never Happen in "Star Trek" #7
-- perl -Mre=debug -e "/just|another|perl|hacker/"
From: Dave Mitchell <davem [...] iabyn.com>
CC: Tony Cook via RT <perl5-security-report [...] perl.org>, rt-deliver-to-perl5-security-report [...] rt.perl.org
Date: Wed, 24 Aug 2016 15:03:05 +0100
Subject: Re: [perl #129012] heap-buffer-overflow Perl_fbm_instr (util.c:974)
To: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 224b
On Wed, Aug 24, 2016 at 03:31:11PM +0200, demerphq wrote: Show quoted text
> Shouldnt that be "intuit" and not "untuit" ?
Yes. A bit late now though :-) -- Modern art: "That's easy, I could have done that!" "Ah, but you didn't!"
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 762b
On Wed Aug 24 06:11:00 2016, davem wrote: Show quoted text
> On Tue, Aug 23, 2016 at 08:57:40AM +0100, Dave Mitchell wrote:
> > On Mon, Aug 22, 2016 at 11:08:00PM -0700, Tony Cook via RT wrote:
> > > It looks like the second time fbm_instr() is called an out of range > > > value for bigend is supplied. Otherwise I haven't worked out > > > what's > > > going on.
> > > > This is an area of code that I've worked on before, so I'll have a > > look.
> > Now fixed with the following. Turns out its not a security concern.
I'm wondering where the boundaries are on what we consider a security issue. In this case if bigend is beyond the end of the allocated block, and that happens to be unmapped memory, this might be used in a denial-of-service attack (crashing perl.) Tony
CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #129012] heap-buffer-overflow Perl_fbm_instr (util.c:974)
Date: Thu, 8 Sep 2016 09:43:44 +0100
To: Tony Cook via RT <perl5-security-report [...] perl.org>
Download (untitled) / with headers
text/plain 1.7k
On Tue, Sep 06, 2016 at 06:03:20PM -0700, Tony Cook via RT wrote: Show quoted text
> On Wed Aug 24 06:11:00 2016, davem wrote:
> > On Tue, Aug 23, 2016 at 08:57:40AM +0100, Dave Mitchell wrote:
> > > On Mon, Aug 22, 2016 at 11:08:00PM -0700, Tony Cook via RT wrote:
> > > > It looks like the second time fbm_instr() is called an out of range > > > > value for bigend is supplied. Otherwise I haven't worked out > > > > what's > > > > going on.
> > > > > > This is an area of code that I've worked on before, so I'll have a > > > look.
> > > > Now fixed with the following. Turns out its not a security concern.
> > I'm wondering where the boundaries are on what we consider a security issue. > > In this case if bigend is beyond the end of the allocated block, and that > happens to be unmapped memory, this might be used in a denial-of-service > attack (crashing perl.)
Well until fairly recently (v5.17.4-76-g7016d6e) the regex engine wouldn't even work with mmapped strings (it assumed amongst other things that the string would always have a trailing \0). So there is unlikely to be much production code out there using patterns against mmapped strings. Also, if you're passing mmapped strings to the regex engine, then you're likely to be searching for binary data, so are unlikely to be using a UTF-8 pattern against it. Yes, there *could* in theory be circumstances where this would allow a DoS attack, but its fairly unlikely. In principle *all* perl bugs are security issues, but it's a subjective judgement as to how exploitable a bug is. I don't think this one is very exploitable. I'm also a lot less worried about DoS than arbitrary code execution, for example. -- "Procrastination grows to fill the available time" -- Mitchell's corollary to Parkinson's Law
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On Thu, 08 Sep 2016 01:44:09 -0700, davem wrote: Show quoted text
> On Tue, Sep 06, 2016 at 06:03:20PM -0700, Tony Cook via RT wrote:
> > I'm wondering where the boundaries are on what we consider a security > > issue. > > > > In this case if bigend is beyond the end of the allocated block, and > > that > > happens to be unmapped memory, this might be used in a denial-of- > > service > > attack (crashing perl.)
> > Well until fairly recently (v5.17.4-76-g7016d6e) the regex engine > wouldn't > even work with mmapped strings (it assumed amongst other things that > the > string would always have a trailing \0). So there is unlikely to be > much > production code out there using patterns against mmapped strings. > > Also, if you're passing mmapped strings to the regex engine, then > you're > likely to be searching for binary data, so are unlikely to be using a > UTF-8 pattern against it.
In glibc, malloc() will allocate memory with mmap() instead of from a heap for large blocks so normal PVs can be memory mapped. There's no need for strange magic from an application. Show quoted text
> Yes, there *could* in theory be circumstances where this would allow a > DoS > attack, but its fairly unlikely. > > In principle *all* perl bugs are security issues, but it's a > subjective > judgement as to how exploitable a bug is. I don't think this one is > very > exploitable. I'm also a lot less worried about DoS than arbitrary > code execution, for example.
I agree, I was just wondering where we place the boundary between "this is a security issue" and "this is not a security issue". In any case, the fix is public, so I've made this ticket public. The issue is fixed, so closing it too. Tony
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been resolved. Perl 5.26.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.26.0 If you find that the problem persists, feel free to reopen this ticket.


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