add a C23 inspired checked integer multiplication helper

Compilers had become far more creative about optimizing code that might be considered undefined behaviour, so improve our integer overflow checking to prevent any possible miscompilations. While at it, expand the use of the checks to pcre2test to fix a related problem.
PCRE2Project · Jan 25, 2023 · 9617440 · 9617440
1 parent 69d6b24
commit 9617440
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 23 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -610,6  610,7 @@ SET(PCRE2_HEADERS ${PROJECT_BINARY_DIR}/pcre2.h)
 SET(PCRE2_SOURCES
   src/pcre2_auto_possess.c
   ${PROJECT_BINARY_DIR}/pcre2_chartables.c
   src/pcre2_chkdint.c
   src/pcre2_compile.c
   src/pcre2_config.c
   src/pcre2_context.c

diff --git a/Makefile.am b/Makefile.am
@@ -353,6  353,7 @@ NODIST_SOURCES = src/pcre2_chartables.c
 
 COMMON_SOURCES = \
   src/pcre2_auto_possess.c \
   src/pcre2_chkdint.c \
   src/pcre2_compile.c \
   src/pcre2_config.c \
   src/pcre2_context.c \

diff --git a/configure.ac b/configure.ac
@@ -638,6  638,8 @@ fi
 if test "$enable_debug" = "yes"; then
   AC_DEFINE([PCRE2_DEBUG], [], [
     Define to any value to include debugging code.])
 else
   CFLAGS="$CFLAGS -DNDEBUG"
 fi
 
 if test "$enable_percent_zt" = "no"; then

diff --git a/src/pcre2_chkdint.c b/src/pcre2_chkdint.c
@@ -0,0  1,79 @@
 /*************************************************
 *      Perl-Compatible Regular Expressions       *
 *************************************************/
 
 /* PCRE is a library of functions to support regular expressions whose syntax
 and semantics are as close as possible to those of the Perl 5 language.
 
                      Written by Philip Hazel
             Copyright (c) 2023 University of Cambridge
 
 -----------------------------------------------------------------------------
 Redistribution and use in source and binary forms, with or without
 modification, are permitted provided that the following conditions are met:
 
     * Redistributions of source code must retain the above copyright notice,
       this list of conditions and the following disclaimer.
 
     * Redistributions in binary form must reproduce the above copyright
       notice, this list of conditions and the following disclaimer in the
       documentation and/or other materials provided with the distribution.
 
     * Neither the name of the University of Cambridge nor the names of its
       contributors may be used to endorse or promote products derived from
       this software without specific prior written permission.
 
 THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
 AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
 LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
 CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
 SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
 INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
 CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 POSSIBILITY OF SUCH DAMAGE.
 -----------------------------------------------------------------------------
 */
 
 /* This file contains functions to implement checked integer operation */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
 
 #include "pcre2_internal.h"
 
 /*************************************************
 *        Checked Integer Multiplication          *
 *************************************************/
 
 /*
 Arguments:
   r         A pointer to PCRE2_SIZE to store the answer
   a, b      Two positive integers
 
 Returns:    Bool indicating if the operation overflows
 
 It is modeled after C23's <stdckdint.h> interface
 The INT64_OR_DOUBLE type is a 64-bit integer type when available,
 otherwise double. */
 
 PCRE2_EXP_DEFN BOOL
 PRIV(ckd_smul)(PCRE2_SIZE *r, int a, int b)
 {
 INT64_OR_DOUBLE m;
 
 #ifndef NDEBUG
 if (a < 0 || b < 0) abort();
 #endif
 
 m = (INT64_OR_DOUBLE)a * (INT64_OR_DOUBLE)b;
 if (!(PCRE2_SIZE_MAX - m > 0)) return TRUE;
 
 *r = (PCRE2_SIZE)m;
 return FALSE;
 }
 
 /* End of pcre_chkdint.c */
diff --git a/src/pcre2_compile.c b/src/pcre2_compile.c
@@ -7112,15  7112,12 @@ for (;; pptr  )
 
         /* In the pre-compile phase, we don't actually do the replication. We
         just adjust the length as if we had. Do some paranoid checks for
-        potential integer overflow. The INT64_OR_DOUBLE type is a 64-bit
-        integer type when available, otherwise double. */
         potential integer overflow. */
 
         if (lengthptr != NULL)
           {
-          PCRE2_SIZE delta = replicate*(1   LINK_SIZE);
-          if ((INT64_OR_DOUBLE)replicate*
-                (INT64_OR_DOUBLE)(1   LINK_SIZE) >
-                  (INT64_OR_DOUBLE)INT_MAX ||
           PCRE2_SIZE delta;
           if (PRIV(ckd_smul)(&delta, replicate, 1   LINK_SIZE) ||
               OFLOW_MAX - *lengthptr < delta)
             {
             *errorcodeptr = ERR20;
@@ -7282,15  7279,12 @@ for (;; pptr  )
             {
             /* In the pre-compile phase, we don't actually do the replication.
             We just adjust the length as if we had. Do some paranoid checks for
-            potential integer overflow. The INT64_OR_DOUBLE type is a 64-bit
-            integer type when available, otherwise double. */
             potential integer overflow. */
 
             if (lengthptr != NULL)
               {
-              PCRE2_SIZE delta = (repeat_min - 1)*length_prevgroup;
-              if ((INT64_OR_DOUBLE)(repeat_min - 1)*
-                    (INT64_OR_DOUBLE)length_prevgroup >
-                      (INT64_OR_DOUBLE)INT_MAX ||
               PCRE2_SIZE delta;
               if (PRIV(ckd_smul)(&delta, repeat_min - 1, length_prevgroup) ||
                   OFLOW_MAX - *lengthptr < delta)
                 {
                 *errorcodeptr = ERR20;
@@ -7334,21  7328,19 @@ for (;; pptr  )
           just adjust the length as if we had. For each repetition we must add
           1 to the length for BRAZERO and for all but the last repetition we
           must add 2   2*LINKSIZE to allow for the nesting that occurs. Do some
-          paranoid checks to avoid integer overflow. The INT64_OR_DOUBLE type
-          is a 64-bit integer type when available, otherwise double. */
           paranoid checks to avoid integer overflow. */
 
           if (lengthptr != NULL && repeat_max > 0)
             {
-            PCRE2_SIZE delta = repeat_max*(length_prevgroup   1   2   2*LINK_SIZE) -
-                        2 - 2*LINK_SIZE;   /* Last one doesn't nest */
-            if ((INT64_OR_DOUBLE)repeat_max *
-                  (INT64_OR_DOUBLE)(length_prevgroup   1   2   2*LINK_SIZE)
-                    > (INT64_OR_DOUBLE)INT_MAX ||
             PCRE2_SIZE delta;
             if (PRIV(ckd_smul)(&delta, repeat_max,
                                length_prevgroup   1   2   2*LINK_SIZE) ||
                 OFLOW_MAX - *lengthptr < delta)
               {
               *errorcodeptr = ERR20;
               return 0;
               }
             delta -= (2 - 2*LINK_SIZE);   /* Last one doesn't nest */
             *lengthptr  = delta;
             }
 

diff --git a/src/pcre2_internal.h b/src/pcre2_internal.h
@@ -156,8  156,8 @@ pcre2_match() because of the way it backtracks. */
 #define PCRE2_SPTR CUSTOM_SUBJECT_PTR
 #endif
 
-/* When checking for integer overflow in pcre2_compile(), we need to handle
-large integers. If a 64-bit integer type is available, we can use that.
 /* When checking for integer overflow, we need to handle large integers.
 If a 64-bit integer type is available, we can use that.
 Otherwise we have to cast to double, which of course requires floating point
 arithmetic. Handle this by defining a macro for the appropriate type. */
 
@@ -2042,6  2042,9 @@ extern void *       _pcre2_memmove(void *, const void *, size_t);
 #endif
 
 #endif  /* PCRE2_CODE_UNIT_WIDTH */
 
 extern BOOL         _pcre2_ckd_smul(PCRE2_SIZE *, int, int);
 
 #endif  /* PCRE2_INTERNAL_H_IDEMPOTENT_GUARD */
 
 /* End of pcre2_internal.h */
diff --git a/src/pcre2test.c b/src/pcre2test.c
@@ -6836,7  6836,7 @@ if (dbuffer != NULL)
 the number of code units that will be needed (though the buffer may have to be
 extended if replication is involved). */
 
-needlen = (size_t)((len 1) * code_unit_size);
 needlen = (len 1) * code_unit_size;
 if (dbuffer == NULL || needlen >= dbuffer_size)
   {
   while (needlen >= dbuffer_size)
@@ -6867,6  6867,7 @@ while ((c = *p  ) != 0)
 
   if (c == ']' && start_rep != NULL)
     {
     PCRE2_SIZE d;
     long li;
     char *endptr;
 
@@ -6898,7  6899,12 @@ while ((c = *p  ) != 0)
       }
 
     replen = CAST8VAR(q) - start_rep;
-    needlen  = replen * i;
     if (_pcre2_ckd_smul(&d, replen, i))
       {
       fprintf(outfile, "** Expanded content too large\n");
       return PR_OK;
       }
     needlen  = d;
 
     if (needlen >= dbuffer_size)
       {