heap-buffer-overflow Perl_fbm_instr (util.c:974) #15534

p5pRT · 2016-08-20T07:24:38Z

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

Searchable as RT129012$

p5pRT · 2016-08-20T07:24:38Z

From @geeknik

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

p5pRT · 2016-08-20T07:24:38Z

From @geeknik

test303.gz

p5pRT · 2016-08-23T06:08:00Z

From @tonycoz

On Sat Aug 20 00:24:38 2016, brian.carpenter@gmail.com wrote:

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

p5pRT · 2016-08-23T06:08:00Z

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

p5pRT · 2016-08-23T07:58:05Z

From @iabyn

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.

--
My get-up-and-go just got up and went.

p5pRT · 2016-08-24T13:11:00Z

From @iabyn

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 71a9d10
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

p5pRT · 2016-08-24T13:31:43Z

From @demerphq

Shouldnt that be "intuit" and not "untuit" ?

On 24 August 2016 at 15:10, Dave Mitchell <davem@iabyn.com> 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.

commit 71a9d10
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/"

p5pRT · 2016-08-24T14:03:32Z

From @iabyn

On Wed, Aug 24, 2016 at 03:31:11PM +0200, demerphq wrote:

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!"

p5pRT · 2016-09-07T01:03:19Z

From @tonycoz

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.)

Tony

p5pRT · 2016-09-08T08:44:09Z

From @iabyn

On Tue, Sep 06, 2016 at 06:03:20PM -0700, Tony Cook via RT wrote:

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

p5pRT · 2017-01-23T04:53:38Z

From @tonycoz

On Thu, 08 Sep 2016 01:44:09 -0700, davem wrote:

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.

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

p5pRT · 2017-01-23T04:53:40Z

@tonycoz - Status changed from 'open' to 'pending release'

p5pRT · 2017-05-30T20:14:48Z

From @khwilliamson

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.

p5pRT · 2017-05-30T20:14:48Z

@khwilliamson - Status changed from 'pending release' to 'resolved'

p5pRT closed this as completed May 30, 2017

heap-buffer-overflow Perl_fbm_instr (util.c:974) #15534

heap-buffer-overflow Perl_fbm_instr (util.c:974) #15534

p5pRT commented Aug 20, 2016

p5pRT commented Aug 20, 2016

p5pRT commented Aug 20, 2016

p5pRT commented Aug 23, 2016

p5pRT commented Aug 23, 2016

p5pRT commented Aug 23, 2016

p5pRT commented Aug 24, 2016

p5pRT commented Aug 24, 2016

p5pRT commented Aug 24, 2016

p5pRT commented Sep 7, 2016

p5pRT commented Sep 8, 2016

p5pRT commented Jan 23, 2017

p5pRT commented Jan 23, 2017

p5pRT commented May 30, 2017

p5pRT commented May 30, 2017

heap-buffer-overflow Perl_fbm_instr (util.c:974) #15534

heap-buffer-overflow Perl_fbm_instr (util.c:974) #15534

Comments

p5pRT commented Aug 20, 2016

p5pRT commented Aug 20, 2016

From @geeknik

p5pRT commented Aug 20, 2016

From @geeknik

p5pRT commented Aug 23, 2016

From @tonycoz

p5pRT commented Aug 23, 2016

p5pRT commented Aug 23, 2016

From @iabyn

p5pRT commented Aug 24, 2016

From @iabyn

p5pRT commented Aug 24, 2016

From @demerphq

p5pRT commented Aug 24, 2016

From @iabyn

p5pRT commented Sep 7, 2016

From @tonycoz

p5pRT commented Sep 8, 2016

From @iabyn

p5pRT commented Jan 23, 2017

From @tonycoz

p5pRT commented Jan 23, 2017

p5pRT commented May 30, 2017

From @khwilliamson

p5pRT commented May 30, 2017