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

signed integer overflow in S_study_chunk (regcomp.c:5444) #16173

Closed
p5pRT opened this issue Sep 26, 2017 · 9 comments
Closed

signed integer overflow in S_study_chunk (regcomp.c:5444) #16173

p5pRT opened this issue Sep 26, 2017 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 26, 2017

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

Searchable as RT132164$

@p5pRT
Copy link
Author

p5pRT commented Sep 26, 2017

From @geeknik

Triggered while fuzzing v5.27.4-28-g60dfa51

./perl -e 'm m0*0+\Rm'

regcomp.c​:5444​:26​: runtime error​: signed integer overflow​:
9223372036854775807 + 1 cannot be represented in type 'long'
  #0 0xc36484 in S_study_chunk /root/perl/regcomp.c​:5444​:26
  #1 0xb7c063 in Perl_re_op_compile /root/perl/regcomp.c​:7574​:11
  #2 0x567839 in Perl_pmruntime /root/perl/op.c​:5888​:6
  #3 0xaf495b in Perl_yyparse /root/perl/perly.y​:1210​:23
  #4 0x7289f7 in S_parse_body /root/perl/perl.c​:2450​:9
  #5 0x714363 in perl_parse /root/perl/perl.c​:1753​:2
  #6 0x50af99 in main /root/perl/perlmain.c​:121​:18
  #7 0x7fd350558b44 in __libc_start_main
/build/glibc-6V9RKT/glibc-2.19/csu/libc-start.c​:287
  #8 0x43c01b in _start (/root/perl/perl+0x43c01b)

SUMMARY​: UndefinedBehaviorSanitizer​: undefined-behavior regcomp.c​:5444​:26

@p5pRT
Copy link
Author

p5pRT commented Sep 26, 2017

From @geeknik

Whoops, that should be "signed" not unsigned.

@p5pRT
Copy link
Author

p5pRT commented Apr 1, 2018

From @khwilliamson

On Mon, 25 Sep 2017 23​:19​:46 -0700, brian.carpenter@​gmail.com wrote​:

Whoops, that should be "signed" not unsigned.

The attached patch fixes this, and seems reasonable. The value is getting initialized to effective infinity, and then incremented. Just above, there is a special case to not incrment infinity, and it seems proper to do the same thing here.

But since I don't understand study_chunk(), I won't apply the patch until the middle of April, to give people a chance to say that it shouldn't be applied.

--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Apr 1, 2018

From @khwilliamson

0006-regcomp.c-Don-t-try-to-increment-infinity.patch
From 2c03b53a90c951ba9aef715d6b8a0ec91fe55994 Mon Sep 17 00:00:00 2001
From: Karl Williamson <khw@cpan.org>
Date: Sun, 1 Apr 2018 13:58:47 -0600
Subject: [PATCH 6/6] regcomp.c: Don't try to increment infinity

This value can be infinity (which is here SSize_t_MAX).  Leave it there.
---
 regcomp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/regcomp.c b/regcomp.c
index 018d5646fc..47e86933f7 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -5476,7 +5476,9 @@ Perl_re_printf( aTHX_  "LHS=%" UVuf " RHS=%" UVuf "\n",
                 /* Cannot expect anything... */
                 scan_commit(pRExC_state, data, minlenp, is_inf);
     	        data->pos_min += 1;
-	        data->pos_delta += 1;
+                if (data->pos_delta != SSize_t_MAX) {
+                    data->pos_delta += 1;
+                }
 		data->cur_is_floating = 1; /* float */
     	    }
 	}
-- 
2.11.0

@p5pRT
Copy link
Author

p5pRT commented Apr 1, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Apr 17, 2018

From @khwilliamson

Fixed by commit
5e20fb0
--
Karl Williamson

@p5pRT
Copy link
Author

p5pRT commented Apr 17, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release yesterday of Perl 5.28.0, this and 185 other issues have been
resolved.

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

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

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant