heap uaf in Perl_yylex() - toke.c:5143 #15878
Comments
From @geeknikTriggered with Perl v5.25.10 (v5.25.9-154-gd8f2fe0) while fuzzing with AFL. ==30048==ERROR: AddressSanitizer: heap-use-after-free on address 0x62500000790b is located 11 bytes inside of 8200-byte region previously allocated by thread T0 here: SUMMARY: AddressSanitizer: heap-use-after-free |
From @geeknik |
From @hvdsOn Sat, 18 Feb 2017 22:20:53 -0800, brian.carpenter@gmail.com wrote:
Short-form reproducer is: The critical part of the stacktrace for the realloc is: There are multiple opportunities for PL_lineptr to get realloced while we're checking for the "Missing $ on loop variable" diagnostic, and we need to update 's' in that case before it is used by the OPERATOR() macro. I have a fix for this, just putting a test together. Hugo |
|
The RT System itself - Status changed from 'new' to 'open' |
From @hvdsOn Sun, 19 Feb 2017 03:04:21 -0800, hv wrote:
Does the below seem like a sensible place to add the test? If so, I'll apply the patches attached after checking a couple of test runs. Hugo Inline Patchdiff --git a/t/comp/parser_run.t b/t/comp/parser_run.t
new file mode 100644
index 0000000..2543f49
--- /dev/null
+++ b/t/comp/parser_run.t
@@ -0,0 +1,28 @@
+#!./perl
+
+# Parser tests that want test.pl, eg to use runperl() for tests to show
+# reads through invalid pointers.
+# Note that this should still be runnable under miniperl.
[...] |
From @hvds0001-perl-130814-Add-testcase-and-new-testfile-t-comp-par.patchFrom ec0e84fc5b799950ae441739c1ed0ae3ead07185 Mon Sep 17 00:00:00 2001
From: Hugo van der Sanden <hv@crypt.org>
Date: Sun, 19 Feb 2017 11:15:38 +0000
Subject: [PATCH 1/2] [perl #130814] Add testcase, and new testfile
t/comp/parser_run.t
Sometimes it's useful to have test.pl around, but it seems inappropriate
to pollute the existing t/comp/parser.t with that.
---
t/comp/parser_run.t | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100644 t/comp/parser_run.t
diff --git a/t/comp/parser_run.t b/t/comp/parser_run.t
new file mode 100644
index 0000000..2543f49
--- /dev/null
+++ b/t/comp/parser_run.t
@@ -0,0 +1,28 @@
+#!./perl
+
+# Parser tests that want test.pl, eg to use runperl() for tests to show
+# reads through invalid pointers.
+# Note that this should still be runnable under miniperl.
+
+BEGIN {
+ @INC = qw(. ../lib );
+ chdir 't' if -d 't';
+}
+
+require './test.pl';
+plan(1);
+
+# [perl #130814] can reallocate lineptr while looking ahead for
+# "Missing $ on loop variable" diagnostic.
+my $result = runperl(
+ prog => " foreach m0\n\$" . ("0" x 0x2000),
+ stderr => 1,
+);
+is($result, <<EXPECT);
+syntax error at -e line 3, near "foreach m0
+"
+Identifier too long at -e line 3.
+EXPECT
+
+__END__
+# ex: set ts=8 sts=4 sw=4 et:
--
2.10.2
|
From @hvds0002-perl-130814-update-pointer-into-PL_linestr-after-loo.patchFrom 4f0e3ba399282a4329994bbff5c4fa75b0a961d0 Mon Sep 17 00:00:00 2001
From: Hugo van der Sanden <hv@crypt.org>
Date: Sun, 19 Feb 2017 10:46:09 +0000
Subject: [PATCH 2/2] [perl #130814] update pointer into PL_linestr after
lookahead
Looking ahead for the "Missing $ on loop variable" diagnostic can reallocate
PL_linestr, invalidating our pointer. Save the offset so we can update it
in that case.
---
toke.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/toke.c b/toke.c
index 5a711d3..cf3163e 100644
--- a/toke.c
+++ b/toke.c
@@ -7913,6 +7913,7 @@ Perl_yylex(pTHX)
&& isIDFIRST_lazy_if_safe(s, PL_bufend, UTF))
{
char *p = s;
+ SSize_t s_off = s - SvPVX(PL_linestr);
if ((PL_bufend - p) >= 3
&& strEQs(p, "my") && isSPACE(*(p + 2)))
@@ -7930,6 +7931,9 @@ Perl_yylex(pTHX)
}
if (*p != '$' && *p != '\\')
Perl_croak(aTHX_ "Missing $ on loop variable");
+
+ /* The buffer may have been reallocated, update s */
+ s = SvPVX(PL_linestr) + s_off;
}
OPERATOR(FOR);
--
2.10.2
|
From @hvdsOn Sun, 19 Feb 2017 03:22:29 -0800, hv wrote:
Silence has implied consent, now pushed as: d7186ad [perl #130814] Add testcase, and new testfile t/comp/parser_run.t Hugo |
|
@hvds - Status changed from 'open' to 'pending release' |
From @jkeenanOn Tue, 21 Feb 2017 12:00:55 GMT, hv wrote:
The humans were silent but the machines were not. In http://perl5.test-smoke.org/report/54176, we got: ##### I manually added t/comp/parser_run.t to MANIFEST and ran 'make test_porting'. I then got failures in t/porting/test_bootstrap.t. These I corrected in: ##### Please review. Thank you very much. |
|
@jkeenan - Status changed from 'pending release' to 'open' |
From @hvdsOn Tue, 21 Feb 2017 07:43:53 -0800, jkeenan wrote:
Thanks, I saw this on the smoke results but you got in there quicker than I could. Your changes look good to me; I've additionally tweaked the comment about exceptions in test_bootstrap. Hugo |
From @craigberryOn Sun, Feb 19, 2017 at 5:22 AM, Hugo van der Sanden via RT
How long does that token need to be to exercise the bug? The total |
From @hvdsOn Fri, 24 Feb 2017 14:48:34 -0800, craig.a.berry@gmail.com wrote:
It looks like 0x1ffb is the first multiple to trigger valgrind, which doesn't help you. Since it's a regression test that's only expected to show failure under valgrind, and only if we mess up and reintroduce the bug, I think it'd be entirely reasonable to skip it on VMS. Alternatively, since there's nothing about the bug that requires the code to be supplied via -e, it could just be put in a progfile. I don't see a function (or runperl() param) that'll do that for you and deal with the cleanup, but I could see such a thing being handy (especially if miniperl-compatible). Hugo |
From @hvdsOn Fri, 24 Feb 2017 17:27:10 -0800, hv wrote:
.. because I'm blind. The attached should do it, I reckon. Hugo |
From @hvds0001-fix-VMS-test-fail.patchFrom 31bdffadc0a3947b09513a413a55cbfe82a2b59a Mon Sep 17 00:00:00 2001
From: Hugo van der Sanden <hv@crypt.org>
Date: Sat, 25 Feb 2017 10:42:17 +0000
Subject: [PATCH] fix VMS test fail
d7186add added a runperl() test that breaks command line length limits for
VMS. Switch to fresh_perl() instead, so the prog is put in a file for us.
---
t/comp/parser_run.t | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/comp/parser_run.t b/t/comp/parser_run.t
index 2543f49..e74644d 100644
--- a/t/comp/parser_run.t
+++ b/t/comp/parser_run.t
@@ -14,14 +14,14 @@ plan(1);
# [perl #130814] can reallocate lineptr while looking ahead for
# "Missing $ on loop variable" diagnostic.
-my $result = runperl(
- prog => " foreach m0\n\$" . ("0" x 0x2000),
- stderr => 1,
+my $result = fresh_perl(
+ " foreach m0\n\$" . ("0" x 0x2000),
+ { stderr => 1 },
);
-is($result, <<EXPECT);
-syntax error at -e line 3, near "foreach m0
+is($result . "\n", <<EXPECT);
+syntax error at - line 3, near "foreach m0
"
-Identifier too long at -e line 3.
+Identifier too long at - line 3.
EXPECT
__END__
--
2.10.2
|
From @craigberryOn Sat, Feb 25, 2017 at 4:47 AM, Hugo van der Sanden via RT
fresh_perl() indeed does the trick and your patch tests out for me. Thanks. |
|
@hvds - 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#130814 (status was 'resolved')
Searchable as RT130814$
The text was updated successfully, but these errors were encountered: