Multiple suspicious Valgrind errors #15585
Comments
From @dcollinsnThe following "script" causes a host of Valgrind warnings, starting with a $ perl -e 'print "exec a00\$"' | valgrind ../bin/perl |
From @dcollinsnAFL crash explorer reports that replacing "exec" with any of the following grep Several similar cases involving the following strings were also identified: flock in general, it appears that this is the repro case: perl -e 'printf "%-7s_\$", "flock"' | valgrind ../bin/perl In other words, exactly 7 characters consisting of a builtin rightpadded by This seems to be so tight that it's unlikely to be exploitable. I'll let it On Sat, Sep 3, 2016 at 9:45 PM, <perl5-security-report@perl.org> wrote:
|
From @tonycozOn Sat Sep 03 21:26:50 2016, dcollinsn@gmail.com wrote:
...
Does the attached fix all your test cases for this? As this involves feeding code to the perl parser, I don't think it's Tony |
From @tonycoz0001-perl-129190-intuit_method-can-move-the-line-buffer.patchFrom e36eaa0b2f687d532fe3b2f0b0bbded8e8a1fa17 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Thu, 8 Sep 2016 13:21:02 +1000
Subject: (perl #129190) intuit_method() can move the line buffer
and broke PL_bufptr when it did.
---
t/op/lex.t | 5 ++++-
toke.c | 10 +++++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/t/op/lex.t b/t/op/lex.t
index a667183..6eac888 100644
--- a/t/op/lex.t
+++ b/t/op/lex.t
@@ -7,7 +7,7 @@ use warnings;
BEGIN { chdir 't' if -d 't'; require './test.pl'; }
-plan(tests => 30);
+plan(tests => 31);
{
no warnings 'deprecated';
@@ -241,3 +241,6 @@ fresh_perl_is(
{},
'[perl #129069] - "Missing name" warning and valgrind clean'
);
+
+fresh_perl_like('flock _$', qr/Not enough arguments for flock/, {stderr => 1},
+ "[perl #129190] intuit_method() invalidates PL_bufptr");
diff --git a/toke.c b/toke.c
index 3ade32b..3779387 100644
--- a/toke.c
+++ b/toke.c
@@ -4079,11 +4079,12 @@ S_intuit_method(pTHX_ char *start, SV *ioname, CV *cv)
}
if (*start == '$') {
+ SSize_t start_off = start - SvPVX(PL_linestr);
if (cv || PL_last_lop_op == OP_PRINT || PL_last_lop_op == OP_SAY
|| isUPPER(*PL_tokenbuf))
return 0;
s = skipspace(s);
- PL_bufptr = start;
+ PL_bufptr = SvPVX(PL_linestr) + start_off;
PL_expect = XREF;
return *s == '(' ? FUNCMETH : METHOD;
}
@@ -7034,17 +7035,24 @@ Perl_yylex(pTHX)
== OA_FILEREF))
{
bool immediate_paren = *s == '(';
+ SSize_t s_off;
/* (Now we can afford to cross potential line boundary.) */
s = skipspace(s);
/* Two barewords in a row may indicate method call. */
+ /* intuit_method() can indirectly call lex_next_chunk(),
+ * invalidating s
+ */
+ s_off = s - SvPVX(PL_linestr);
if ((isIDFIRST_lazy_if(s,UTF) || *s == '$')
&& (tmp = intuit_method(s, lex ? NULL : sv, cv)))
{
+ /* the code at method: doesn't use s */
goto method;
}
+ s = SvPVX(PL_linestr) + s_off;
/* If not a declared subroutine, it's an indirect object. */
/* (But it's an indir obj regardless for sort.) */
--
2.1.4
|
|
The RT System itself - Status changed from 'new' to 'open' |
From @dcollinsnSorry for the delay in responding to this. Yes, Tony, the patch you On Wed, Sep 7, 2016 at 11:23 PM, Tony Cook via RT <
|
From @iabynOn Sun, Sep 11, 2016 at 01:02:44AM -0400, Dan Collins wrote:
Tony, any particular reason you haven't applied your patch yet? -- |
From @tonycozOn Mon, 12 Dec 2016 07:54:39 -0800, davem wrote:
I lost track of it. Applied as 743e3e7 (with some noise.) Since this isn't a security issue, the ticket is now public, and closed since it's patched. Tony |
|
@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#129190 (status was 'resolved')
Searchable as RT129190$
The text was updated successfully, but these errors were encountered: