This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
PATCH: [perl #133871] heap-buffer-overflow in S_reginsert
authorKarl Williamson <khw@cpan.org>
Thu, 14 Mar 2019 01:11:46 +0000 (19:11 -0600)
committerKarl Williamson <khw@cpan.org>
Thu, 14 Mar 2019 01:27:49 +0000 (19:27 -0600)
The regex compiler was written assuming it knew how many parentheses
pairs there were at code generation time.  When I converted to a single
pass in 7c932d07cab18751bfc7515b4320436273a459e2, most things were
straight forward to not have to know this number, but there were a few
where it was non-trivial (for me anyway) to figure out how to handle.
So I punted on these and do a second pass when these are encountered.
There are few of them and are less commonly used, namely (?R), (?|...)
and forward references to groups (which most commonly will end up being
a syntax error anyway).

The fix in this commit is to avoid doing some parentheses relocations
when a regnode is inserted when it is known that the parentheses counts
are unreliable (during initial parsing of one of these tricky
constructs).  The code in the ticket is using a branch reset '(?|...)'.
A second pass will get done, and the insert properly handled then, after
the counts are reliable.

regcomp.c
t/re/pat.t

index a753d8c..51679e9 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -19550,7 +19550,11 @@ S_reginsert(pTHX_ RExC_state_t *pRExC_state, const U8 op,
     src = REGNODE_p(RExC_emit);
     RExC_emit += size;
     dst = REGNODE_p(RExC_emit);
-    if (RExC_open_parens) {
+
+    /* If we are in a "count the parentheses" pass, the numbers are unreliable,
+     * and [perl #133871] shows this can lead to problems, so skip this
+     * realignment of parens until a later pass when they are reliable */
+    if (! IN_PARENS_PASS && RExC_open_parens) {
         int paren;
         /*DEBUG_PARSE_FMT("inst"," - %" IVdf, (IV)RExC_npar);*/
         /* remember that RExC_npar is rex->nparens + 1,
index e97031f..f1be50a 100644 (file)
@@ -24,7 +24,7 @@ BEGIN {
 skip_all('no re module') unless defined &DynaLoader::boot_DynaLoader;
 skip_all_without_unicode_tables();
 
-plan tests => 854;  # Update this when adding/deleting tests.
+plan tests => 855;  # Update this when adding/deleting tests.
 
 run_tests() unless caller;
 
@@ -2002,6 +2002,10 @@ while( "\N{U+100}bc" =~ /(..?)(?{$^N})/g ) {
 }
 CODE
     }
+    {   # [perl #133871], ASAN/valgrind out-of-bounds access
+;
+        fresh_perl_like('qr/(?|(())|())|//', qr/syntax error/, {}, "[perl #133871]");
+    }
 
 } # End of sub run_tests