Multiple crash with eval

Question

Multiple crash with eval

p5pRT opened this issue 7 years ago · 13 comments

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

Searchable as RT131562$

Answer 1 · 2017-06-13T08:28:29.000Z

From @Mipu94

I just try hongfuzz and found some samples that triggered crash by *eval*
funtion.
Please find file attached bellow to check.

--
Ta Dinh Sung,

Answer 2 · 2017-06-13T08:28:29.000Z

From @Mipu94

samples.tar.gz

Answer 3 · 2017-06-13T11:43:13.000Z

From @iabyn

On Tue, Jun 13, 2017 at 01:28:29AM -0700, sung wrote:

# New Ticket Created by sung
# Please include the string: [perl #131562]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=131562 >

I just try hongfuzz and found some samples that triggered crash by *eval*
funtion.
Please find file attached bellow to check.

Most of these seem to be fixed in blead. With threaded, debugging perl
builds running under valgrind, I see:

poc1 poc2 poc3 poc4 poc5
---- ---- ---- ---- ----
perl 5.24.0 FAIL FAIL FAIL FAIL FAIL
perl 5.26.0 FAIL PASS FAIL PASS FAIL
bleadperl PASS PASS FAIL PASS PASS

So it would appear that all except poc3 have been fixed.
What perl version(s) did you see failures on?

poc3 is:

$^P = 0xA;
eval qq{#line 162335469120778 "figgle"\n#line 85 "doggo"\n};

It doesn't crash with $^P (debugging flags) being set, so I doubt that it
is a security issue.

--
Little fly, thy summer's play my thoughtless hand
has terminated with extreme prejudice.
(with apologies to William Blake)

Answer 4 · 2017-06-13T11:43:14.000Z

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

Answer 5 · 2017-06-13T12:17:03.000Z

From @Mipu94

My perl version is v5.22.1.

2017-06-13 18:43 GMT+07:00 Dave Mitchell via RT <
perl5-security-report@perl.org>:

On Tue, Jun 13, 2017 at 01:28:29AM -0700, sung wrote:

# New Ticket Created by sung
# Please include the string: [perl #131562]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=131562 >

I just try hongfuzz and found some samples that triggered crash by *eval*
funtion.
Please find file attached bellow to check.

Most of these seem to be fixed in blead. With threaded, debugging perl
builds running under valgrind, I see:
        poc1  poc2  poc3  poc4  poc5
        \-\-\-\-  \-\-\-\-  \-\-\-\-  \-\-\-\-  \-\-\-\-
perl 5.24.0 FAIL FAIL FAIL FAIL FAIL
perl 5.26.0 FAIL PASS FAIL PASS FAIL
bleadperl PASS PASS FAIL PASS PASS

So it would appear that all except poc3 have been fixed.
What perl version(s) did you see failures on?

poc3 is:
$^P = 0xA;
eval qq\{\#line 162335469120778 "figgle"\\n\#line 85 "doggo"\\n\};
It doesn't crash with $^P (debugging flags) being set, so I doubt that it
is a security issue.

--
Little fly, thy summer's play my thoughtless hand
has terminated with extreme prejudice.
(with apologies to William Blake)

--
Ta Dinh Sung,

Phone:0905414043
Work :student at University Information of Technology in Ho Chi Minh.

Answer 6 · 2017-08-23T04:19:39.000Z

From @tonycoz

On Tue, 13 Jun 2017 04:43:13 -0700, davem wrote:

On Tue, Jun 13, 2017 at 01:28:29AM -0700, sung wrote:

# New Ticket Created by sung
# Please include the string: [perl #131562]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=131562 >

I just try hongfuzz and found some samples that triggered crash by *eval*
funtion.
Please find file attached bellow to check.

Most of these seem to be fixed in blead. With threaded, debugging perl
builds running under valgrind, I see:
        poc1  poc2  poc3  poc4  poc5
        \-\-\-\-  \-\-\-\-  \-\-\-\-  \-\-\-\-  \-\-\-\-
perl 5.24.0 FAIL FAIL FAIL FAIL FAIL
perl 5.26.0 FAIL PASS FAIL PASS FAIL
bleadperl PASS PASS FAIL PASS PASS

So it would appear that all except poc3 have been fixed.
What perl version(s) did you see failures on?

poc3 is:
$^P = 0xA;
eval qq\{\#line 162335469120778 "figgle"\\n\#line 85 "doggo"\\n\};
It doesn't crash with $^P (debugging flags) being set, so I doubt that it
is a security issue.

This isn't a security issue and is now public (it is a bug).

It occurs while copying eval lines to the new @{"_<newfilename"} array from the old array.

The code takes the very large line number and converts it to an I32 producing a negative number, then passes that as the index to av_store().

The attached should fix it, I don't think a test is practical as
it would require a very large array to store lines above the 2G mark.

Tony

Answer 7 · 2017-08-23T04:19:39.000Z

From @tonycoz

0001-perl-131562-correct-large-line-numbers-copying-eval-.patch

From 4dc2acb7c6f86ab6ae2b157022d7c8cdbcfef1f2 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 23 Aug 2017 14:18:26 +1000
Subject: (perl #131562) correct large line numbers copying eval lines on #line

Previously this used I32 for line numbers, which takes half the range
of line_t and folds it into negative numbers, leading to trying to store
the lines at negative indexes.

The while loop was also modified to stop storing if/when the line number
no longer fits into cop_line, or no longer fits into SSize_t (as a
positive number) since the index parameter to av_store() is a SSize_t.
---
 toke.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/toke.c b/toke.c
index 99b737b..cac88db 100644
--- a/toke.c
+++ b/toke.c
@@ -1817,14 +1817,14 @@ S_incline(pTHX_ const char *s)
 		    }
 		    else if (GvAV(cfgv)) {
 			AV * const av = GvAV(cfgv);
-			const I32 start = CopLINE(PL_curcop)+1;
-			I32 items = AvFILLp(av) - start;
+			const line_t start = CopLINE(PL_curcop)+1;
+			SSize_t items = AvFILLp(av) - start;
 			if (items > 0) {
 			    AV * const av2 = GvAVn(gv2);
 			    SV **svp = AvARRAY(av) + start;
-			    I32 l = (I32)line_num+1;
-			    while (items--)
-				av_store(av2, l++, SvREFCNT_inc(*svp++));
+			    Size_t l = line_num+1;
+			    while (items-- && l < SSize_t_MAX && l == (line_t)l)
+				av_store(av2, (SSize_t)l++, SvREFCNT_inc(*svp++));
 			}
 		    }
 		}
-- 
2.1.4

Answer 8 · 2017-09-28T18:03:45.000Z

From @Mipu94

I think this is a security bug because this bug can write out of bound. So
we get the permission write, this bug can lead to remote code execution.
This bug cause crash program through argument of eval function (denied of
service).

2017-08-23 11:19 GMT+07:00 Tony Cook via RT <perlbug-followup@perl.org>:

On Tue, 13 Jun 2017 04:43:13 -0700, davem wrote:
On Tue, Jun 13, 2017 at 01:28:29AM -0700, sung wrote:

# New Ticket Created by sung
# Please include the string: [perl #131562]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=131562 >

I just try hongfuzz and found some samples that triggered crash by
*eval*
funtion.
Please find file attached bellow to check.

Most of these seem to be fixed in blead. With threaded, debugging perl
builds running under valgrind, I see:
        poc1  poc2  poc3  poc4  poc5
        \-\-\-\-  \-\-\-\-  \-\-\-\-  \-\-\-\-  \-\-\-\-
perl 5.24.0 FAIL FAIL FAIL FAIL FAIL
perl 5.26.0 FAIL PASS FAIL PASS FAIL
bleadperl PASS PASS FAIL PASS PASS

So it would appear that all except poc3 have been fixed.
What perl version(s) did you see failures on?

poc3 is:
$^P = 0xA;
eval qq\{\#line 162335469120778 "figgle"\\n\#line 85 "doggo"\\n\};
It doesn't crash with $^P (debugging flags) being set, so I doubt that it
is a security issue.
This isn't a security issue and is now public (it is a bug).

It occurs while copying eval lines to the new @{"_<newfilename"} array
from the old array.

The code takes the very large line number and converts it to an I32
producing a negative number, then passes that as the index to av_store().

The attached should fix it, I don't think a test is practical as
it would require a very large array to store lines above the 2G mark.

Tony

From 4dc2acb7c6f86ab6ae2b157022d7c8cdbcfef1f2 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 23 Aug 2017 14:18:26 +1000
Subject: (perl #131562) correct large line numbers copying eval lines on
#line

Previously this used I32 for line numbers, which takes half the range
of line_t and folds it into negative numbers, leading to trying to store
the lines at negative indexes.

The while loop was also modified to stop storing if/when the line number
no longer fits into cop_line, or no longer fits into SSize_t (as a
positive number) since the index parameter to av_store() is a SSize_t.
---
toke.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/toke.c b/toke.c
index 99b737b..cac88db 100644
--- a/toke.c
+++ b/toke.c
@@ -1817,14 +1817,14 @@ S_incline(pTHX_ const char *s)
}
else if (GvAV(cfgv)) {
AV * const av = GvAV(cfgv);
- const I32 start = CopLINE(PL_curcop)+1;
- I32 items = AvFILLp(av) - start;
+ const line_t start = CopLINE(PL_curcop)+1;
+ SSize_t items = AvFILLp(av) - start;
if (items > 0) {
AV * const av2 = GvAVn(gv2);
SV **svp = AvARRAY(av) + start;
- I32 l = (I32)line_num+1;
- while (items--)
- av_store(av2, l++, SvREFCNT_inc(*svp++));
+ Size_t l = line_num+1;
+ while (items-- && l < SSize_t_MAX && l ==
(line_t)l)
+ av_store(av2, (SSize_t)l++,
SvREFCNT_inc(*svp++));
}
}
}
--
2.1.4

--
Ta Dinh Sung,

Answer 9 · 2019-01-21T03:37:29.000Z

From @tonycoz

On Thu, 28 Sep 2017 11:03:45 -0700, tadinhsung@gmail.com wrote:

I think this is a security bug because this bug can write out of
bound. So
we get the permission write, this bug can lead to remote code
execution.
This bug cause crash program through argument of eval function (denied
of
service).

Sorry, I missed this follow up.

If an attacker can feed code to eval, they can feed code like C< system "rm -rf /" >, making other bugs irrelevant.

Tony

Answer 10 · 2019-01-21T04:13:40.000Z

From @tonycoz

On Tue, 22 Aug 2017 21:19:39 -0700, tonyc wrote:

This isn't a security issue and is now public (it is a bug).

It occurs while copying eval lines to the new @{"_<newfilename"} array
from the old array.

The code takes the very large line number and converts it to an I32
producing a negative number, then passes that as the index to
av_store().

The attached should fix it, I don't think a test is practical as
it would require a very large array to store lines above the 2G mark.

Applied as 515c395.

Tony

Answer 11 · 2019-01-21T04:13:40.000Z

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

Answer 12 · 2019-05-22T15:05:36.000Z

From @khwilliamson

Thank 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
resolved.

Perl 5.30.0 may be downloaded via:
https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists, feel free to reopen this ticket.

Answer 13 · 2019-05-22T15:05:36.000Z

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