[pcre-dev] [Bug 2382] New: [10.33-RC1] Crash in pcre2_subst…

https://bugs.exim.org/show_bug.cgi?id=2382

            Bug ID: 2382
           Summary: [10.33-RC1] Crash in pcre2_substitute_32() if called
                    with a NULL mcontext
           Product: PCRE
           Version: N/A
          Hardware: x86
                OS: Linux
            Status: NEW
          Severity: bug
          Priority: medium
         Component: Code
          Assignee: ph10@???
          Reporter: ppisar@???
                CC: pcre-dev@???

I was reported that fish-3.0.2, a shell implementation, started to crash after
upgrading PCRE2 from 10.32 to 10.33-RC1
<https://bugzilla.redhat.com/show_bug.cgi?id=1686434>.

The fish executable uses UTF-32 interface and calls pcre2_substitute_32() like
this:

        pcre2_rc = pcre2_substitute(regex.code, PCRE2_SPTR(arg.c_str()),
arglen,
                                    0,  // start offset
                                    options, regex.match,
                                    0,  // match context
                                    PCRE2_SPTR(replacement->c_str()),
replacement->length(),
                                    (PCRE2_UCHAR *)output, &outlen);

The important thing is the 7th argument (named mcontext in PCRE2 API) is NULL.
Since this PCRE2 commit:

commit 3c2c4493cc3b12dddd2493b465f0ce50e3f77b5a
Author: ph10 <ph10@6239d852-aaf2-0410-a92c-79f79f948069>
Date: Tue Sep 18 16:31:30 2018 +0000

    Implement callouts from pcre2_substitute().
    git-svn-id: svn://vcs.exim.org/pcre2/code/trunk@1012
6239d852-aaf2-0410-a92c-79f79f948069

that brought substitute callouts, fish crashes in pcre2_substitute_32() at:

/* The replacement has been copied to the output, or its size has been
remembered. Do the callout if there is one and we have done an actual
replacement. */

→ if (!overflowed && mcontext->substitute_callout != NULL)
    {
    scb.subscount = subs;
    scb.output_offsets[1] = buff_offset;
    rc = mcontext->substitute_callout(&scb, mcontext->substitute_callout_data);

because the mcontext pointer passed by the application is NULL. The mcontext
value is used twice in the pcre_substitute() function. The first use falls back
to code's context.

I believe that a proper fix is skip the substitute callouts if mcontext is
NULL:

--- 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;

--
You are receiving this mail because:
You are on the CC list for the bug.