From e85de98d0a0c7872b3dbd790ae3f6ced55d3fabe Mon Sep 17 00:00:00 2001 From: "Philip.Hazel" Date: Mon, 11 Mar 2019 17:29:08 +0000 Subject: [PATCH] Fix crash in pcre2_substitute() with NULL match context. --- ChangeLog | 8 +++++--- doc/html/pcre2test.html | 18 ++++++++++-------- doc/pcre2test.1 | 20 +++++++++++--------- doc/pcre2test.txt | 18 ++++++++++-------- src/pcre2_substitute.c | 2 +- src/pcre2test.c | 24 +++++++++++++----------- testdata/testinput2 | 8 ++++++++ testdata/testoutput2 | 12 ++++++++++++ 8 files changed, 70 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index b9f1b96..331cb2f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,8 +2,8 @@ Change Log for PCRE2 -------------------- -Version 10.33-RC1 03-March-2019 -------------------------------- +Version 10.33 11-March-2019 +--------------------------- 1. Added "allvector" to pcre2test to make it easy to check the part of the ovector that shouldn't be changed, in particular after substitute and failed or @@ -12,7 +12,9 @@ partial matches. 2. Fix subject buffer overread in JIT when UTF is disabled and \X or \R has a greater than 1 fixed quantifier. This issue was found by Yunho Kim. -3. Added support for callouts from pcre2_substitute(). +3. Added support for callouts from pcre2_substitute(). After 10.33-RC1, but +prior to release, fixed a bug that caused a crash if pcre2_substitute() was +called with a NULL match context. 4. The POSIX functions are now all called pcre2_regcomp() etc., with wrapper functions that use the standard POSIX names. However, in pcre2posix.h the POSIX diff --git a/doc/html/pcre2test.html b/doc/html/pcre2test.html index 8f35acc..083d5cc 100644 --- a/doc/html/pcre2test.html +++ b/doc/html/pcre2test.html @@ -1450,8 +1450,10 @@ Testing substitute callouts

If the substitute_callout modifier is set, a substitution callout -function is set up. When it is called (after each substitution), details of the -the input and output strings are output. For example: +function is set up. The null_context modifier must not be set, because +the address of the callout function is passed in a match context. When the +callout function is called (after each substitution), details of the the input +and output strings are output. For example: 

   /abc/g,replace=<$0>,substitute_callout
       abcdefabcpqr
@@ -1626,11 +1628,11 @@ Passing a NULL context
 

 

 Normally, pcre2test passes a context block to pcre2_match(),
-pcre2_dfa_match() or pcre2_jit_match(). If the null_context
-modifier is set, however, NULL is passed. This is for testing that the matching
-functions behave correctly in this case (they use default values). This
-modifier cannot be used with the find_limits modifier or when testing the
-substitution function.
+pcre2_dfa_match(), pcre2_jit_match() or pcre2_substitute().
+If the null_context modifier is set, however, NULL is passed. This is for
+testing that the matching and substitution functions behave correctly in this
+case (they use default values). This modifier cannot be used with the
+find_limits or substitute_callout modifiers.
 

 
THE ALTERNATIVE MATCHING FUNCTION

 

@@ -2076,7 +2078,7 @@ Cambridge, England.
 

 
REVISION

 

-Last updated: 11 February 2019
+Last updated: 11 March 2019
 

 Copyright © 1997-2019 University of Cambridge.
 

diff --git a/doc/pcre2test.1 b/doc/pcre2test.1
index 954e043..568e705 100644
--- a/doc/pcre2test.1
+++ b/doc/pcre2test.1
@@ -1,4 +1,4 @@
-.TH PCRE2TEST 1 "11 February 2019" "PCRE 10.33"
+.TH PCRE2TEST 1 "11 March 2019" "PCRE 10.33"
 .SH NAME
 pcre2test - a program for testing Perl-compatible regular expressions.
 .SH SYNOPSIS
@@ -1417,8 +1417,10 @@ matching provokes an error return ("bad option value") from
 .rs
 .sp
 If the \fBsubstitute_callout\fP modifier is set, a substitution callout
-function is set up. When it is called (after each substitution), details of the
-the input and output strings are output. For example:
+function is set up. The \fBnull_context\fP modifier must not be set, because
+the address of the callout function is passed in a match context. When the
+callout function is called (after each substitution), details of the the input
+and output strings are output. For example:
 .sp
   /abc/g,replace=<$0>,substitute_callout
       abcdefabcpqr
@@ -1587,11 +1589,11 @@ passing the replacement string as zero-terminated.
 .rs
 .sp
 Normally, \fBpcre2test\fP passes a context block to \fBpcre2_match()\fP,
-\fBpcre2_dfa_match()\fP or \fBpcre2_jit_match()\fP. If the \fBnull_context\fP
-modifier is set, however, NULL is passed. This is for testing that the matching
-functions behave correctly in this case (they use default values). This
-modifier cannot be used with the \fBfind_limits\fP modifier or when testing the
-substitution function.
+\fBpcre2_dfa_match()\fP, \fBpcre2_jit_match()\fP or \fBpcre2_substitute()\fP.
+If the \fBnull_context\fP modifier is set, however, NULL is passed. This is for
+testing that the matching and substitution functions behave correctly in this
+case (they use default values). This modifier cannot be used with the
+\fBfind_limits\fP or \fBsubstitute_callout\fP modifiers.
 .
 .
 .SH "THE ALTERNATIVE MATCHING FUNCTION"
@@ -2057,6 +2059,6 @@ Cambridge, England.
 .rs
 .sp
 .nf
-Last updated: 11 February 2019
+Last updated: 11 March 2019
 Copyright (c) 1997-2019 University of Cambridge.
 .fi
diff --git a/doc/pcre2test.txt b/doc/pcre2test.txt
index ca3b7e3..cbe3528 100644
--- a/doc/pcre2test.txt
+++ b/doc/pcre2test.txt
@@ -1303,8 +1303,10 @@ SUBJECT MODIFIERS
    Testing substitute callouts
 
        If the substitute_callout modifier is set, a substitution callout func-
-       tion is set up. When it is called (after each substitution), details of
-       the the input and output strings are output. For example:
+       tion  is set up. The null_context modifier must not be set, because the
+       address of the callout function is passed in a match context. When  the
+       callout  function  is  called (after each substitution), details of the
+       the input and output strings are output. For example:
 
          /abc/g,replace=<$0>,substitute_callout
              abcdefabcpqr
@@ -1457,11 +1459,11 @@ SUBJECT MODIFIERS
    Passing a NULL context
 
        Normally,  pcre2test  passes  a   context   block   to   pcre2_match(),
-       pcre2_dfa_match() or pcre2_jit_match(). If the null_context modifier is
-       set, however, NULL is passed. This is for  testing  that  the  matching
-       functions behave correctly in this case (they use default values). This
-       modifier cannot be used with the find_limits modifier or  when  testing
-       the substitution function.
+       pcre2_dfa_match(),  pcre2_jit_match()  or  pcre2_substitute().   If the
+       null_context modifier is set, however, NULL  is  passed.  This  is  for
+       testing  that  the matching and substitution functions behave correctly
+       in this case (they use default values). This modifier  cannot  be  used
+       with the find_limits or substitute_callout modifiers.
 
 
 THE ALTERNATIVE MATCHING FUNCTION
@@ -1888,5 +1890,5 @@ AUTHOR
 
 REVISION
 
-       Last updated: 11 February 2019
+       Last updated: 11 March 2019
        Copyright (c) 1997-2019 University of Cambridge.
diff --git a/src/pcre2_substitute.c b/src/pcre2_substitute.c
index 0b46629..0b63ef4 100644
--- a/src/pcre2_substitute.c
+++ b/src/pcre2_substitute.c
@@ -839,7 +839,7 @@ do
   remembered. Do the callout if there is one and we have done an actual 
   replacement. */
   
-  if (!overflowed && mcontext->substitute_callout != NULL)
+  if (!overflowed && mcontext != NULL && mcontext->substitute_callout != NULL)
     {
     scb.subscount = subs;  
     scb.output_offsets[1] = buff_offset;
diff --git a/src/pcre2test.c b/src/pcre2test.c
index e493099..46722ef 100644
--- a/src/pcre2test.c
+++ b/src/pcre2test.c
@@ -1388,13 +1388,13 @@ are supported. */
 
 #define PCRE2_SUBSTITUTE(a,b,c,d,e,f,g,h,i,j,k,l) \
   if (test_mode == PCRE8_MODE) \
-    a = pcre2_substitute_8(G(b,8),(PCRE2_SPTR8)c,d,e,f,G(g,8),G(h,8), \
+    a = pcre2_substitute_8(G(b,8),(PCRE2_SPTR8)c,d,e,f,G(g,8),h, \
       (PCRE2_SPTR8)i,j,(PCRE2_UCHAR8 *)k,l); \
   else if (test_mode == PCRE16_MODE) \
-    a = pcre2_substitute_16(G(b,16),(PCRE2_SPTR16)c,d,e,f,G(g,16),G(h,16), \
+    a = pcre2_substitute_16(G(b,16),(PCRE2_SPTR16)c,d,e,f,G(g,16),h, \
       (PCRE2_SPTR16)i,j,(PCRE2_UCHAR16 *)k,l); \
   else \
-    a = pcre2_substitute_32(G(b,32),(PCRE2_SPTR32)c,d,e,f,G(g,32),G(h,32), \
+    a = pcre2_substitute_32(G(b,32),(PCRE2_SPTR32)c,d,e,f,G(g,32),h, \
       (PCRE2_SPTR32)i,j,(PCRE2_UCHAR32 *)k,l)
 
 #define PCRE2_SUBSTRING_COPY_BYNAME(a,b,c,d,e) \
@@ -1866,11 +1866,11 @@ the three different cases. */
 #define PCRE2_SUBSTITUTE(a,b,c,d,e,f,g,h,i,j,k,l) \
   if (test_mode == G(G(PCRE,BITONE),_MODE)) \
     a = G(pcre2_substitute_,BITONE)(G(b,BITONE),(G(PCRE2_SPTR,BITONE))c,d,e,f, \
-      G(g,BITONE),G(h,BITONE),(G(PCRE2_SPTR,BITONE))i,j, \
+      G(g,BITONE),h,(G(PCRE2_SPTR,BITONE))i,j, \
       (G(PCRE2_UCHAR,BITONE) *)k,l); \
   else \
     a = G(pcre2_substitute_,BITTWO)(G(b,BITTWO),(G(PCRE2_SPTR,BITTWO))c,d,e,f, \
-      G(g,BITTWO),G(h,BITTWO),(G(PCRE2_SPTR,BITTWO))i,j, \
+      G(g,BITTWO),h,(G(PCRE2_SPTR,BITTWO))i,j, \
       (G(PCRE2_UCHAR,BITTWO) *)k,l)
 
 #define PCRE2_SUBSTRING_COPY_BYNAME(a,b,c,d,e) \
@@ -2068,7 +2068,7 @@ the three different cases. */
   pcre2_set_substitute_callout_8(G(a,8), \
     (int (*)(pcre2_substitute_callout_block_8 *, void *))b,c)
 #define PCRE2_SUBSTITUTE(a,b,c,d,e,f,g,h,i,j,k,l) \
-  a = pcre2_substitute_8(G(b,8),(PCRE2_SPTR8)c,d,e,f,G(g,8),G(h,8), \
+  a = pcre2_substitute_8(G(b,8),(PCRE2_SPTR8)c,d,e,f,G(g,8),h, \
     (PCRE2_SPTR8)i,j,(PCRE2_UCHAR8 *)k,l)
 #define PCRE2_SUBSTRING_COPY_BYNAME(a,b,c,d,e) \
   a = pcre2_substring_copy_byname_8(G(b,8),G(c,8),(PCRE2_UCHAR8 *)d,e)
@@ -2175,7 +2175,7 @@ the three different cases. */
   pcre2_set_substitute_callout_16(G(a,16), \
     (int (*)(pcre2_substitute_callout_block_16 *, void *))b,c)
 #define PCRE2_SUBSTITUTE(a,b,c,d,e,f,g,h,i,j,k,l) \
-  a = pcre2_substitute_16(G(b,16),(PCRE2_SPTR16)c,d,e,f,G(g,16),G(h,16), \
+  a = pcre2_substitute_16(G(b,16),(PCRE2_SPTR16)c,d,e,f,G(g,16),h, \
     (PCRE2_SPTR16)i,j,(PCRE2_UCHAR16 *)k,l)
 #define PCRE2_SUBSTRING_COPY_BYNAME(a,b,c,d,e) \
   a = pcre2_substring_copy_byname_16(G(b,16),G(c,16),(PCRE2_UCHAR16 *)d,e)
@@ -2282,7 +2282,7 @@ the three different cases. */
   pcre2_set_substitute_callout_32(G(a,32), \
     (int (*)(pcre2_substitute_callout_block_32 *, void *))b,c)
 #define PCRE2_SUBSTITUTE(a,b,c,d,e,f,g,h,i,j,k,l) \
-  a = pcre2_substitute_32(G(b,32),(PCRE2_SPTR32)c,d,e,f,G(g,32),G(h,32), \
+  a = pcre2_substitute_32(G(b,32),(PCRE2_SPTR32)c,d,e,f,G(g,32),h, \
     (PCRE2_SPTR32)i,j,(PCRE2_UCHAR32 *)k,l)
 #define PCRE2_SUBSTRING_COPY_BYNAME(a,b,c,d,e) \
   a = pcre2_substring_copy_byname_32(G(b,32),G(c,32),(PCRE2_UCHAR32 *)d,e)
@@ -6939,11 +6939,13 @@ for (k = 0; k < sizeof(exclusive_dat_controls)/sizeof(uint32_t); k++)
 
 if (pat_patctl.replacement[0] != 0)
   {
-  if ((dat_datctl.control & CTL_NULLCONTEXT) != 0)
+  if ((dat_datctl.control2 & CTL2_SUBSTITUTE_CALLOUT) != 0 &&
+      (dat_datctl.control & CTL_NULLCONTEXT) != 0)
     {
-    fprintf(outfile, "** Replacement text is not supported with null_context.\n");
+    fprintf(outfile, "** Replacement callouts are not supported with null_context.\n");
     return PR_OK;
     }
+
   if ((dat_datctl.control & CTL_ALLCAPTURES) != 0)
     fprintf(outfile, "** Ignored with replacement text: allcaptures\n");
   }
@@ -7335,7 +7337,7 @@ if (dat_datctl.replacement[0] != 0)
     }
 
   PCRE2_SUBSTITUTE(rc, compiled_code, pp, arg_ulen, dat_datctl.offset,
-    dat_datctl.options|xoptions, match_data, dat_context,
+    dat_datctl.options|xoptions, match_data, use_dat_context,
     rbuffer, rlen, nbuffer, &nsize);
 
   if (rc < 0)
diff --git a/testdata/testinput2 b/testdata/testinput2
index 433f9f8..9e59b62 100644
--- a/testdata/testinput2
+++ b/testdata/testinput2
@@ -5579,4 +5579,12 @@ a)"xI
 
 /(*ACCEPT:XX)^abc/I
 
+/abc/replace=xyz
+    abc\=null_context
+
+/abc/replace=xyz,substitute_callout
+    abc 
+\= Expect error message
+    abc\=null_context
+
 # End of testinput2
diff --git a/testdata/testoutput2 b/testdata/testoutput2
index d0e289d..2f91c38 100644
--- a/testdata/testoutput2
+++ b/testdata/testoutput2
@@ -16922,6 +16922,18 @@ Subject length lower bound = 3
 Capture group count = 0
 Subject length lower bound = 0
 
+/abc/replace=xyz
+    abc\=null_context
+ 1: xyz
+
+/abc/replace=xyz,substitute_callout
+    abc 
+ 1(1) Old 0 3 "abc" New 0 3 "xyz"
+ 1: xyz
+\= Expect error message
+    abc\=null_context
+** Replacement callouts are not supported with null_context.
+
 # End of testinput2
 Error -70: PCRE2_ERROR_BADDATA (unknown error number)
 Error -62: bad serialized data