pcre2test: refactor replication implementations

Make the pattern implemenation aware of the underlying buffer
size to avoid mostly harmless buffer overreads, and allow for
a nicer fallback in case of syntax errors.

Update the data implementation to behave similarly when the
provided number is not valid, and cleanup fixes that had
accumlated unorganically.

Author: carenas

doc/pcre2test.1

@@ -563,7 +563,9 @@ part of the file. For example:
   \e[abc]{4}
 .sp
 is converted to "abcabcabcabc". This feature does not support nesting. To
-include a closing square bracket in the characters, code it as \ex5D.
+include a closing square bracket in the characters, code it with \ex followed
+by two hexadecimal digits that represent that letter in the character set used
+(e.g. \ex5D for ASCII or UTF-8).
 .P
 A backslash followed by an equals sign marks the end of the subject string and
 the start of a modifier list. For example:

src/pcre2test.c

@@ -5975,8 +5975,7 @@ static int
 process_pattern(void)
 {
 BOOL utf;
-uint32_t k;
-uint8_t *p = buffer;
+uint8_t *pend, *p = buffer;
 unsigned int delimiter = *p++;
 int errorcode;
 void *use_pat_context;
@@ -6029,6 +6028,7 @@ if (p[1] == '\\') *p++ = '\\';
 
 *p++ = 0;
 patlen = p - buffer - 2;
+pend = buffer + 1 + patlen;
 
 /* Look for modifiers and options after the final delimiter. */
 
@@ -6068,7 +6068,7 @@ if (pat_patctl.convert_type != CONVERT_UNSET &&
 /* Check for mutually exclusive control modifiers. At present, these are all in
 the first control word. */
 
-for (k = 0; k < sizeof(exclusive_pat_controls)/sizeof(uint32_t); k++)
+for (uint32_t k = 0; k < sizeof(exclusive_pat_controls)/sizeof(uint32_t); k++)
   {
   uint32_t c = pat_patctl.control & exclusive_pat_controls[k];
   if (c != 0 && c != (c & (~c+1)))
@@ -6166,54 +6166,63 @@ else if ((pat_patctl.control & CTL_EXPAND) != 0)
     uint8_t *pc = pp;
     uint32_t count = 1;
     size_t length = 1;
+    size_t m = 1;
 
     /* Check for replication syntax; if not found, the defaults just set will
-    prevail and one character will be copied. */
+    prevail and `length` characters will be copied once. */
 
-    if (pp[0] == '\\' && pp[1] == '[')
+    if (memcmp(pp, "\\[", 2) == 0)
       {
       uint8_t *pe;
-      for (pe = pp + 2; *pe != 0; pe++)
+
+      /* Start the capture after skipping the prefix. This pointer will need
+      to be updated back if a problem is found and would rather go through
+      the literal fallback below */
+      pc += 2;
+      for (pe = pc; *pe != 0; pe++)
         {
-        if (pe[0] == ']' && pe[1] == '{')
+        if (pend - pe > 3 && memcmp(pe, "]{", 2) == 0 && isdigit(pe[2]))
           {
-          size_t clen = pe - pc - 2;
-          uint32_t i = 0;
           unsigned long uli;
           char *endptr;
 
-          pe += 2;
-          uli = strtoul((const char *)pe, &endptr, 10);
-          if (U32OVERFLOW(uli))
+          errno = 0;
+          uli = strtoul((const char *)pe + 2, &endptr, 10);
+          if (errno != 0 || uli == 0 || U32OVERFLOW(uli))
             {
-            fprintf(outfile, "** Pattern repeat count too large\n");
+            fprintf(outfile, "** Invalid replication count (1..UINT_MAX)\n");
             return PR_SKIP;
             }
 
-          i = (uint32_t)uli;
-          pe = (uint8_t *)endptr;
-          if (*pe == '}')
+          if (*endptr == '}') count = (uint32_t)uli;
+          length = pe - pc;
+          if (length >= SIZE_MAX/count)
             {
-            if (i == 0)
-              {
-              fprintf(outfile, "** Zero repeat not allowed\n");
-              return PR_SKIP;
-              }
-            pc += 2;
-            count = i;
-            length = clen;
-            pp = pe;
-            break;
+            fprintf(outfile, "** Expanded content too large\n");
+            return PR_SKIP;
             }
+          pe = (uint8_t *)endptr;
+          break;
           }
         }
+      if (*pe != '}')
+        {
+        pc -= 2;
+        length = pe - pc;
+        }
+      m = length * count;
+      pp = pe;
+
+      /* The main loop increments pp, so if we are already at the end of
+      the pattern need to backtrack to avoid jumping over the NUL. */
+      if (*pe == 0) pp--;
       }
 
     /* Add to output. If the buffer is too small expand it. The function for
     expanding buffers always keeps buffer and pbuffer8 in step as far as their
     size goes. */
 
-    while (pt + count * length > pbuffer8 + pbuffer8_size)
+    while (pt + m > pbuffer8 + pbuffer8_size)
       {
       size_t pc_offset = pc - buffer;
       size_t pp_offset = pp - buffer;
@@ -7958,7 +7967,7 @@ process_data(void)
 {
 PCRE2_SIZE len, ulen, arg_ulen;
 uint32_t gmatched;
-uint32_t c, k;
+uint32_t c;
 uint32_t g_notempty = 0;
 uint8_t *p, *pp, *start_rep;
 size_t needlen;
@@ -8089,14 +8098,15 @@ while ((c = *p++) != 0)
 
     if (*p++ != '{')
       {
-      fprintf(outfile, "** Expected '{' after \\[....]\n");
+      fprintf(outfile, "** Expected '{' after \\[...]\n");
       return PR_OK;
       }
 
+    errno = 0;
     li = strtol((const char *)p, &endptr, 10);
-    if (S32OVERFLOW(li))
+    if (!isdigit(*p) || errno != 0 || li < 1 || S32OVERFLOW(li))
       {
-      fprintf(outfile, "** Repeat count too large\n");
+      fprintf(outfile, "** Replication count missing or invalid (1..INT_MAX)\n");
       return PR_OK;
       }
     i = (int)li;
@@ -8108,44 +8118,41 @@ while ((c = *p++) != 0)
       return PR_OK;
       }
 
-    if (i-- <= 0)
+    if (i-- > 1)
       {
-      fprintf(outfile, "** Zero or negative repeat not allowed\n");
-      return PR_OK;
-      }
-
-    replen = CAST8VAR(q) - start_rep;
-    if (i > 0 && replen > (SIZE_MAX - needlen) / i)
-      {
-      fprintf(outfile, "** Expanded content too large\n");
-      return PR_OK;
-      }
-    needlen += replen * i;
-
-    if (needlen >= dbuffer_size)
-      {
-      size_t qoffset = CAST8VAR(q) - dbuffer;
-      size_t rep_offset = start_rep - dbuffer;
-      while (needlen >= dbuffer_size)
+      replen = CAST8VAR(q) - start_rep;
+      if (replen >= (SIZE_MAX - needlen) / i)
         {
-        if (dbuffer_size < SIZE_MAX/2) dbuffer_size *= 2;
-          else dbuffer_size = needlen + 1;
+        fprintf(outfile, "** Expanded content too large\n");
+        return PR_OK;
         }
-      dbuffer = (uint8_t *)realloc(dbuffer, dbuffer_size);
-      if (dbuffer == NULL)
+      needlen += replen * i;
+
+      if (needlen >= dbuffer_size)
         {
-        fprintf(stderr, "pcre2test: realloc(%" SIZ_FORM ") failed\n",
-          dbuffer_size);
-        exit(1);
+        size_t qoffset = CAST8VAR(q) - dbuffer;
+        size_t rep_offset = start_rep - dbuffer;
+        while (needlen >= dbuffer_size)
+          {
+          if (dbuffer_size < SIZE_MAX / 2) dbuffer_size *= 2;
+            else dbuffer_size = needlen + 1;
+          }
+        dbuffer = (uint8_t *)realloc(dbuffer, dbuffer_size);
+        if (dbuffer == NULL)
+          {
+          fprintf(stderr, "pcre2test: realloc(%" SIZ_FORM ") failed\n",
+            dbuffer_size);
+          exit(1);
+          }
+        SETCASTPTR(q, dbuffer + qoffset);
+        start_rep = dbuffer + rep_offset;
         }
-      SETCASTPTR(q, dbuffer + qoffset);
-      start_rep = dbuffer + rep_offset;
-      }
 
-    while (i-- > 0)
-      {
-      memcpy(CAST8VAR(q), start_rep, replen);
-      SETPLUS(q, replen/code_unit_size);
+      while (i-- > 0)
+        {
+        memcpy(CAST8VAR(q), start_rep, replen);
+        SETPLUS(q, replen/code_unit_size);
+        }
       }
 
     start_rep = NULL;
@@ -8426,7 +8433,7 @@ if (dat_datctl.substitute_skip != 0 || dat_datctl.substitute_stop != 0)
 /* Check for mutually exclusive modifiers. At present, these are all in the
 first control word. */
 
-for (k = 0; k < sizeof(exclusive_dat_controls)/sizeof(uint32_t); k++)
+for (uint32_t k = 0; k < sizeof(exclusive_dat_controls)/sizeof(uint32_t); k++)
   {
   c = dat_datctl.control & exclusive_dat_controls[k];
   if (c != 0 && c != (c & (~c+1)))

testdata/testinput2

@@ -4589,9 +4589,6 @@
 /(abc)*/
     \[abc]{1}
 
-/(abc)*/
-    \[abc]{0}
-
 /^/gm
     \n\n\n
 
@@ -5136,12 +5133,29 @@ a)"xI
 \= Expect no match
     abc
 
+# Expand tests
+
+/(abc)*/
+    \[abc]{0}
+    \[abc]{}
+
 /aaa/
-\[abc]{10000000000000000000000000000}
-\[a]{3}
+    \[X]{-10}
+    \[abc]{10000000000000000000000000000}
+    \[a]{3}
 
 /\[AB]{6000000000000000000000}/expand
 
+/\[AB]{0000000000000000000000}/expand
+
+/a\[b]{-1}/BI,expand
+
+/a\[b]{/BI,expand
+
+/a\[/BI,expand
+
+//BI,expand
+
 # Hex uses pattern length, not zero-terminated. This tests for overrunning
 # the given length of a pattern.
 
@@ -6324,9 +6338,6 @@ a)"xI
 /[Aa]{2,3}/BI
     aabcd
 
---
-    \[X]{-10}
-    
 # Check imposition of maximum by match_data_create().
 
 /abcd/

testdata/testoutput2

@@ -15177,10 +15177,6 @@ No match
  0: abc
  1: abc
 
-/(abc)*/
-    \[abc]{0}
-** Zero or negative repeat not allowed
-
 /^/gm
     \n\n\n
  0: 
@@ -16358,14 +16354,77 @@ Failed: error 122 at offset 11: unmatched closing parenthesis
   0    ^    0
 No match
 
+# Expand tests
+
+/(abc)*/
+    \[abc]{0}
+** Replication count missing or invalid (1..INT_MAX)
+    \[abc]{}
+** Replication count missing or invalid (1..INT_MAX)
+
 /aaa/
-\[abc]{10000000000000000000000000000}
-** Repeat count too large
-\[a]{3}
+    \[X]{-10}
+** Replication count missing or invalid (1..INT_MAX)
+    \[abc]{10000000000000000000000000000}
+** Replication count missing or invalid (1..INT_MAX)
+    \[a]{3}
  0: aaa
 
 /\[AB]{6000000000000000000000}/expand
-** Pattern repeat count too large
+** Invalid replication count (1..UINT_MAX)
+
+/\[AB]{0000000000000000000000}/expand
+** Invalid replication count (1..UINT_MAX)
+
+/a\[b]{-1}/BI,expand
+Expanded: a\[b]{-1}
+------------------------------------------------------------------
+        Bra
+        a[b]{-1}
+        Ket
+        End
+------------------------------------------------------------------
+Capture group count = 0
+First code unit = 'a'
+Last code unit = '}'
+Subject length lower bound = 8
+
+/a\[b]{/BI,expand
+Expanded: a\[b]{
+------------------------------------------------------------------
+        Bra
+        a[b]{
+        Ket
+        End
+------------------------------------------------------------------
+Capture group count = 0
+First code unit = 'a'
+Last code unit = '{'
+Subject length lower bound = 5
+
+/a\[/BI,expand
+Expanded: a\[
+------------------------------------------------------------------
+        Bra
+        a[
+        Ket
+        End
+------------------------------------------------------------------
+Capture group count = 0
+First code unit = 'a'
+Last code unit = '['
+Subject length lower bound = 2
+
+//BI,expand
+Expanded: 
+------------------------------------------------------------------
+        Bra
+        Ket
+        End
+------------------------------------------------------------------
+Capture group count = 0
+May match empty string
+Subject length lower bound = 0
 
 # Hex uses pattern length, not zero-terminated. This tests for overrunning
 # the given length of a pattern.
@@ -18979,10 +19038,6 @@ Subject length lower bound = 2
     aabcd
  0: aa
 
---
-    \[X]{-10}
-** Zero or negative repeat not allowed
-    
 # Check imposition of maximum by match_data_create().
 
 /abcd/