bug#15307: minor fix to dfa.c

[Jim Meyering]

On Sun, Sep 8, 2013 at 2:53 AM, Aharon Robbins <address@hidden> wrote:
> The following fix to dfa.c was suggested by a static checking tool.
> I'm applying it in the gawk code base.
>
> Basically, it's theoretically possible for len to have run off the end
> of the `str' array.
...
> diff --git a/dfa.c b/dfa.c
> index 8b79eb7..490a075 100644
> --- a/dfa.c
> +++ b/dfa.c
> @@ -1038,7 +1038,8 @@ parse_bracket_exp (void)
>                      /* This is in any case an invalid class name.  */
>                      str[0] = '\0';
>                  }
> -              str[len] = '\0';
> +              if (len < BRACKET_BUFFER_SIZE)
> +                 str[len] = '\0';
>
>                /* Fetch bracket.  */
>                FETCH_WC (c, wc, _("unbalanced ["));

Hi Arnold,

Thanks, but that makes it look like "str" will instead fail to be
NUL-terminated,
in which case the following strcmp (aka STREQ) would overrun the buffer.
Yes, this is all theoretical, but still...

I see that the current limit is 31:

  $ for i in 30 31 32 33; do printf "$i "; src/grep -E '[[:'$(perl -e
'print "a"x'$i)':]]'; done
  30 src/grep: Invalid character class name
  31 src/grep: Invalid character class name
  32 src/grep: Unmatched [ or [^
  33 src/grep: Unmatched [ or [^

So I propose this patch instead:

>From f1e1fb2c5c1538c313f8488ef687b9a96684f54e Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 8 Sep 2013 10:49:52 -0700
Subject: [PATCH] dfa: appease a static analyzer, and save 95 stack bytes

* src/dfa.c (MAX_BRACKET_STRING_LEN): Rename from BRACKET_BUFFER_SIZE
and decrease from 128 to 32.
(parse_bracket_exp): Add one byte more than MAX_BRACKET_STRING_LEN
to the length of "str" buffer, to avoid appearance that we may store
the trailing NUL beyond the end of buffer.  A string of length 32
or greater is rejected by earlier processing, so would never reach
this code.  Addresses http://bugs.gnu.org/15307
---
 src/dfa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/dfa.c b/src/dfa.c
index ad38d3b..b447a8a 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -975,8 +975,8 @@ parse_bracket_exp (void)
          dfa is ever called.  */
       if (c == '[' && (syntax_bits & RE_CHAR_CLASSES))
         {
-#define BRACKET_BUFFER_SIZE 128
-          char str[BRACKET_BUFFER_SIZE];
+#define MAX_BRACKET_STRING_LEN 32
+          char str[MAX_BRACKET_STRING_LEN + 1];
           FETCH_WC (c1, wc1, _("unbalanced ["));

           /* If pattern contains '[[:', '[[.', or '[[='.  */
@@ -990,7 +990,7 @@ parse_bracket_exp (void)
                   FETCH_WC (c, wc, _("unbalanced ["));
                   if ((c == c1 && *lexptr == ']') || lexleft == 0)
                     break;
-                  if (len < BRACKET_BUFFER_SIZE)
+                  if (len < MAX_BRACKET_STRING_LEN)
                     str[len++] = c;
                   else
                     /* This is in any case an invalid class name.  */
-- 
1.8.4.99.gd2dbd39