libxt_string: Fix array out of bounds check

Commit 56d7ab42f3782 ("libxt_string: Avoid potential array out of bounds
access") tried to fix parse_hex_string() for overlong strings but the
change still allowed for 'sindex' to become XT_STRING_MAX_PATTERN_SIZE
which leads to access of first byte after info->pattern. This is not
really a problem because it merely overwrites info->patlen before
calling xtables_error() later, but covscan still detects it so it's
still worth fixing.

The crucial bit here is that 'sindex' has to be incremented at end of
the last iteration since its value is used for info->patlen. Hence just
move the overflow check to the beginning of the loop.

Fixes: 56d7ab42f3782 ("libxt_string: Avoid potential array out of bounds access")
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/extensions/libxt_string.c b/extensions/libxt_string.c
index d298c6a7081e7ade72233edaee7d762ecb101a54..7c6366cbbf1b39b27d11472814865660836b6c8b 100644 (file)
--- a/extensions/libxt_string.c
+++ b/extensions/libxt_string.c
@@ -103,6 +103,9 @@ parse_hex_string(const char *s, struct xt_string_info *info)
        }
 
        while (i < slen) {
+               if (sindex >= XT_STRING_MAX_PATTERN_SIZE)
+                       xtables_error(PARAMETER_PROBLEM,
+                                     "STRING too long \"%s\"", s);
                if (s[i] == '\\' && !hex_f) {
                        literal_f = 1;
                } else if (s[i] == '\\') {
@@ -159,8 +162,7 @@ parse_hex_string(const char *s, struct xt_string_info *info)
                        info->pattern[sindex] = s[i];
                        i++;
                }
-               if (++sindex > XT_STRING_MAX_PATTERN_SIZE)
-                       xtables_error(PARAMETER_PROBLEM, "STRING too long \"%s\"", s);
+               sindex++;
        }
        info->patlen = sindex;
 }