heap-buffer-overflow Perl_re_intuit_start (regexec.c:809:37) #15554
Comments
From @geeknikThe attached script triggers a heap-buffer-overflow Perl_re_intuit_start ==17057==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60800000b978 is located 0 bytes to the right of 88-byte region SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 __interceptor_memcmp |
From @geeknik |
From @tonycozOn Thu Aug 25 12:04:57 2016, brian.carpenter@gmail.com wrote:
I didn't need libdisallocator to reproduce this. valgrind didn't complain about this, but I guess it's just letting Here's a simplified/shortened version: $_ = "\x{f2}\x{140}\x{fe}\x{ff}\x{ff}\x{ff}"; This caused by Perl_re_intuit_start() attempting to match against the fixed if (slen && (*SvPVX_const(check) != *s Here s is a pointer to the sixth character in $_ (the *last* character), The end of the string is only 2 bytes after s, and we're attemptting to We might argue that we expect memcmp() to step through the two strings byte See also: http://trust-in-soft.com/memcmp-requires-pointers-to-fully-valid-buffers/ Patch attached. It *might* be possible to use this as a hard to trigger denial of service Tony |
From @tonycoz0001-perl-129085-avoid-memcmp-past-the-end-of-a-string.patchFrom 4b772b77a4eb75b062f40cc32018e0f34fac601e Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Mon, 29 Aug 2016 15:04:55 +1000
Subject: (perl #129085) avoid memcmp() past the end of a string
When a match is anchored against the start of a string, the regexp
can be compiled to include a fixed string match against a fixed
offset in the string.
In some cases, where the matched against string included UTF-8 before
the fixed offset, this could result in attempting a memcmp() which
overlaps the end of the string and potentially past the end of the
allocated memory.
---
regexec.c | 5 +++--
t/re/pat_rt_report.t | 9 ++++++++-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/regexec.c b/regexec.c
index fad8876..bba3b81 100644
--- a/regexec.c
+++ b/regexec.c
@@ -805,8 +805,9 @@ Perl_re_intuit_start(pTHX_
/* Now should match s[0..slen-2] */
slen--;
}
- if (slen && (*SvPVX_const(check) != *s
- || (slen > 1 && memNE(SvPVX_const(check), s, slen))))
+ if (slen && (strend - s < slen
+ || *SvPVX_const(check) != *s
+ || (slen > 1 && (memNE(SvPVX_const(check), s, slen)))))
{
DEBUG_EXECUTE_r(Perl_re_printf( aTHX_
" String not equal...\n"));
diff --git a/t/re/pat_rt_report.t b/t/re/pat_rt_report.t
index addb3e2..bee1b19 100644
--- a/t/re/pat_rt_report.t
+++ b/t/re/pat_rt_report.t
@@ -20,7 +20,7 @@ use warnings;
use 5.010;
use Config;
-plan tests => 2501; # Update this when adding/deleting tests.
+plan tests => 2502; # Update this when adding/deleting tests.
run_tests() unless caller;
@@ -1123,6 +1123,13 @@ EOP
ok($s !~ /00000?\x80\x80\x80/, "RT #129012");
}
+ {
+ # RT #129085 heap-buffer-overflow Perl_re_intuit_start
+ # this did fail under ASAN, but didn't under valgrind
+ my $s = "\x{f2}\x{140}\x{fe}\x{ff}\x{ff}\x{ff}";
+ ok($s !~ /^0000.\34500\376\377\377\377/, "RT #129085");
+ }
+
} # End of sub run_tests
1;
--
2.1.4
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @jhiOn Monday-201608-29 1:08, Tony Cook via RT wrote:
This is craziness. Yet another victim in the altar of speed. What is the more general "fix"? (not referring to Tony's spot fix here) |
From @tonycozOn Mon Aug 29 04:42:35 2016, jhi wrote:
This seems like an overreaction, or a reaction to a problem that doesn't exist. The bug ASAN is detecting is that we're supplying a region of memory to A lesser case might be us comparing within the allocated block, but beyond Such an attack probably isn't practical, but I might be wrong. Tony |
From @iabynOn Mon, Aug 29, 2016 at 05:48:19PM -0700, Tony Cook via RT wrote:
Yep, definitely a bug at our end. In theory a match like /ab\0X/
This particular test is intended as a quick fail. The worst that can i.e. in extremely unlikely circumstances, an optimisation is skipped. I don't think this is a security issue any more, and I think your patch -- |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank 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 Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#129085 (status was 'resolved')
Searchable as RT129085$
The text was updated successfully, but these errors were encountered: