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

p5pRT commented 8 years ago

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

Searchable as RT129012$

p5pRT commented 8 years ago

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 commented 8 years ago

From @geeknik

test303.gz

p5pRT commented 8 years ago

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 commented 8 years ago

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

p5pRT commented 8 years ago

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 commented 8 years ago

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

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 7 years ago

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 commented 7 years ago

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

p5pRT commented 7 years ago

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 commented 7 years ago

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

Perl / perl5