-
Notifications
You must be signed in to change notification settings - Fork 563
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
substitution within (?{}) causes segmentation fault #15353
Comments
From [email protected]The following program illustrates the issue: ### BEGIN-CODE ### my $str = "foo"; ### END-CODE ### Tested with perl-5.22.2 and perl-5.25.1, no output is produced, but after about a second, a segmentation fault exception is raised: $ time perl bug.pl |
From [email protected]A somewhat related issue is the following: ### BEGIN-CODE ### m< s//$^R/ee;
//; ### END-CODE ### The output is "Just another Perl hacker", printed 10 times, for each empty match ("//"). |
From [Unknown Contact. See original ticket]A somewhat related issue is the following: ### BEGIN-CODE ### m< s//$^R/ee;
//; ### END-CODE ### The output is "Just another Perl hacker", printed 10 times, for each empty match ("//"). |
From @dcollinsnThis appears to be an overflow of the stack caused by infinite recursion, as evidenced by the following repeating stack frames: #11330 0x00000000006e43f3 in S_regmatch (reginfo=0x7fffffee9860, startpos=0xaba170 "foo", prog=0xaa37a0) at regexec.c:6731 Here is some GDB output from the very beginning of the backtrace: Breakpoint 1, Perl_regexec_flags (rx=0xab2ae0, stringarg=0xaba170 "foo", strend=0xaba173 "", strbeg=0xaba170 "foo", minend=0, sv=0xab29c0, data=0x0, flags=1) at regexec.c:2878 Valgrind agrees that the segfault is caused by the stack overflow: ==64578== Stack overflow in thread #1: can't grow stack to 0xffe801000 **BISECT** This error has been present at least since 5.8.0. |
The RT System itself - Status changed from 'new' to 'open' |
From @demerphqFor what its worth i believe this is safe if you wrap the s/// in a
|
From [email protected]On Mon May 23 14:22:37 2016, demerphq wrote:
Seems like not s/// is the issue here, as the following code, where s/// was replaced with eval(), behaves the same: ### BEGIN-CODE ### m{ //;//;//;//;//; ### END-CODE ### Probably I should open a new ticket for this? |
From [email protected]On Wed May 25 15:33:56 2016, trizenx@gmail.com wrote:
Code simplified to: ### BEGIN-CODE ### m{ //;//;//;//;//; ### END-CODE ### |
From @cpansproutOn Wed May 25 15:42:20 2016, trizenx@gmail.com wrote:
I don�t see what the bug is here. The empty pattern re-uses the last successful match. -- Father Chrysostomos |
From [email protected]On Wed May 25 15:55:20 2016, sprout wrote:
I never heard of this behavior before. Is this officially documented? Personally, I see it as a security issue. For example, consider the following artificial scenario: ### BEGIN-CODE ### /(?{ print "sending money\n" })/x; print "Insert regex: "; ### END-CODE ### If a user inserts a regular expression that happens to coincide with the last regular expression that successfully matched, but also executed some code in (?{}), the same code will be executed again, which is something that I don't think it should happen. In the above scenario, a user can take advantage of this behavior and exploit it in his favor, making it a security hole. |
From @demerphqOn 26 May 2016 at 00:55, Father Chrysostomos via RT
I dont get it either.
I don't get why you bring this up either. Yves |
From @cpansproutOn Wed May 25 16:33:50 2016, trizenx@gmail.com wrote:
perl.git$ ack 'last successful' pod pod/perlop.pod pod/perlretut.pod
You have to use (?:) in cases like that: /(?{ print "sending money\n" })/x; print "Insert regex: ";
Neither do I (at least with /$foo/; with // it should stay as it is), but it is hard to change this because of backward compatibility. That�s a separate issue from your original post. If you want to continue discussing this particular point, please open a new ticket. -- Father Chrysostomos |
From @demerphqOn 25 May 2016 21:03, "Father Chrysostomos via RT" <
Fwiw i dont buy the back compat argument on this one. I have never seen
I agree. But feel free to quote my opinion on it when you do. Yved |
From @iabynOn Mon, May 23, 2016 at 10:48:26AM -0700, Daniel �uteu wrote:
As has been pointed out elsewhere in this ticket, an empty pattern Marking the currently executing pattern as 'last successful' #prints "a" -- |
From @ap* Dave Mitchell <davem@iabyn.com> [2016-05-30 15:09]:
Is it possible to detect this case and die instead? It wouldn�t do what |
From @cpansproutOn Mon May 30 05:08:27 2016, davem wrote:
Does the last-successful-match logic use PL_curpm? -- Father Chrysostomos |
From @iabynOn Mon, May 30, 2016 at 10:18:38AM -0700, Father Chrysostomos via RT wrote:
Yes. -- |
From @iabynOn Mon, May 30, 2016 at 03:49:06PM +0200, Aristotle Pagaltzis wrote:
The following seems to detect it for pp_match() while not failing anything I'm not sure if there are mutual recursion scenarios which could still $ perl5240 -e'"a" =~ /(?{ m{} })/' Inline Patchdiff --git a/pp_hot.c b/pp_hot.c
index 223169b..5292383 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -1767,6 +1767,8 @@ PP(pp_match)
if (!ReANY(rx)->mother_re && !RX_PRELEN(rx)
&& PL_curpm) {
pm = PL_curpm;
+ if (pm == PL_reg_curpm)
+ Perl_croak(aTHX_ "panic: XXX curpm recursion\n");
rx = PM_GETRE(pm);
}
-- Overhead, without any fuss, the stars were going out. |
From @cpansproutOn Mon May 30 13:17:08 2016, davem wrote:
I think that�s a good idea, but that it should not be a panic message, since a panic suggests that perl is not functioning correctly. Perhaps �Use of last successful match is not supported within regexp code blocks�. But I see from perldiag that we call them �eval-groups� in existing messages. -- Father Chrysostomos |
From @ap* Dave Mitchell <davem@iabyn.com> [2016-05-30 22:22]:
I thought not but actually it�s trivial. This program segfaults: $a = qr/(?{ m{$b} })/; |
Migrated from rt.perl.org#128225 (status was 'open')
Searchable as RT128225$
The text was updated successfully, but these errors were encountered: