On Sun, Sep 8, 2013 at 2:53 AM, Aharon Robbins <[email protected]> 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 <[email protected]> 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
