heap-buffer-overflow in S_scan_const (toke.c:4103) #16983
Comments
From @dur-randirCreated by @dur-randirWhile fuzzing perl v5.29.10-23-g7c0d7520a3 built with afl and run 00000000 79 20 6f 5c 78 7b 31 30 30 7d c4 8c ff ff 80 80 |y o\x{100}......| To trigger heap-buffer-overflow write ASAN diagnostics: WRITE of size 1 at 0x603000000c21 thread T0 0x603000000c21 is located 0 bytes to the right of 17-byte region This is a regression in blead, bisect points to the following commit, commit 7d6e74d toke.c: Streamline a case When we are parsing a constant, and the source and destination differ in Perl Info |
From @dur-randir |
From @khwilliamsonOn 4/27/19 10:28 AM, Sergey Aleynikov (via RT) wrote:
The attached patches fix this. I think this bug has been around before The final patch stems from my realization that we could check before |
From @khwilliamson0004-XXX-test-PATCH-perl-134067-heap-buffer-overflow-in-l.patchFrom d8a3c082fd2eb2547ec2ca363c00e5928eaf237d Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Sat, 27 Apr 2019 14:04:58 -0600
Subject: [PATCH 4/5] XXX test: PATCH: [perl #134067] heap buffer overflow in
lexing
This bug happens under tr///. In some circumstances, a byte is inserted
in the output that wasn't in the input, and it did not check that there
was space available for this character. The result could be a write
after the buffer end.
I suspect that this bug has been there all along, and the blamed commit
rearranged things so that it is more likely to happen.
---
toke.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/toke.c b/toke.c
index 3d81b01bfd..8e1b542738 100644
--- a/toke.c
+++ b/toke.c
@@ -3194,11 +3194,23 @@ S_scan_const(pTHX_ char *start)
&& (range_min > 255 || ! convert_unicode)
#endif
) {
+ const STRLEN off = d - SvPVX(sv);
+ const STRLEN extra = 1 + (send - s) + 1;
+ char *e;
+
/* Move the high character one byte to the right; then
* insert between it and the range begin, an illegal
* byte which serves to indicate this is a range (using
* a '-' would be ambiguous). */
- char *e = d++;
+
+ if (off + extra > SvLEN(sv)) {
+ STRLEN max_ptr_off = max_ptr - SvPVX(sv);
+
+ d = off + SvGROW(sv, off + extra);
+ max_ptr = d - off + max_ptr_off;
+ }
+
+ e = d++;
while (e-- > max_ptr) {
*(e + 1) = *e;
}
--
2.17.1
|
From @khwilliamson0005-S_scan_const-Make-sure-room-for-NUL-in-dest.patchFrom c79feaa42f42b483cc640aaa0c68025a61f2f543 Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Sat, 27 Apr 2019 14:30:02 -0600
Subject: [PATCH 5/5] S_scan_const: Make sure room for NUL in dest
At the end of constant, we add a trailing NUL. This makes sure there's
room for it. But the code earlier was supposed to have already mad
enough space, so its a bug if there isn't enough space. So on DEBUGGING
builds, we panic, as we've done before. But otherwise we can continue
on with no harm having been done.
---
toke.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/toke.c b/toke.c
index 8e1b542738..08251aaec9 100644
--- a/toke.c
+++ b/toke.c
@@ -4113,12 +4113,31 @@ S_scan_const(pTHX_ char *start)
}
} /* while loop to process each character */
+ {
+ const STRLEN off = d - SvPVX(sv);
+
+ /* See if room for the terminating NUL */
+ if (UNLIKELY(off >= SvLEN(sv))) {
+
+#ifndef DEBUGGING
+
+ if (off > SvLEN(sv))
+#endif
+ Perl_croak(aTHX_ "panic: constant overflowed allocated space,"
+ " %" UVuf " >= %" UVuf, (UV)off, (UV)SvLEN(sv));
+
+ /* Whew! Here we don't have room for the terminating NUL, but
+ * everything else so far has fit. It's not too late to grow
+ * to fit the NUL and continue on. But it is a bug, as the code
+ * above was supposed to have made room for this, so under
+ * DEBUGGING builds, we panic anyway. */
+ d = off + SvGROW(sv, off + 1);
+ }
+ }
+
/* terminate the string and set up the sv */
*d = '\0';
SvCUR_set(sv, d - SvPVX_const(sv));
- if (SvCUR(sv) >= SvLEN(sv))
- Perl_croak(aTHX_ "panic: constant overflowed allocated space, %" UVuf
- " >= %" UVuf, (UV)SvCUR(sv), (UV)SvLEN(sv));
SvPOK_on(sv);
if (d_is_utf8) {
--
2.17.1
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @dur-randirI suggest this to be added to the 5.30 blockers list. |
From @xsawyerx+1 On 5/2/19 1:01 PM, Sergey Aleynikov via RT wrote:
|
From @khwilliamsonFixed by commit 3fdfceb PATCH: [perl #134067] heap buffer overflow in lexing |
|
@khwilliamson - 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.30.0, this and 160 other issues have been Perl 5.30.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#134067 (status was 'resolved')
Searchable as RT134067$
The text was updated successfully, but these errors were encountered: