From e0814905161f8420d3be47dd3883b1f3c9590a82 Mon Sep 17 00:00:00 2001 From: Karl Williamson Date: Wed, 13 Mar 2019 19:11:46 -0600 Subject: [PATCH] PATCH: [perl #133871] heap-buffer-overflow in S_reginsert 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 | 6 +++++- t/re/pat.t | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/regcomp.c b/regcomp.c index a753d8c..51679e9 100644 --- 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, diff --git a/t/re/pat.t b/t/re/pat.t index e97031f..f1be50a 100644 --- a/t/re/pat.t +++ b/t/re/pat.t @@ -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 -- 1.8.3.1