Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple crash with eval #16015

Closed
p5pRT opened this issue Jun 13, 2017 · 13 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jun 13, 2017

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

Searchable as RT131562$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 13, 2017

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,

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 13, 2017

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 13, 2017

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)

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 13, 2017

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 13, 2017

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.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 28, 2017

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,

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2019

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2019

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 21, 2019

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 22, 2019

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.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 22, 2019

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

@p5pRT p5pRT closed this May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.