From 6fc7762f6f78526e3cb0c189ac2778a6be4c00b5 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Mon, 17 Sep 2018 13:38:33 +0200 Subject: 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 Signed-off-by: Pablo Neira Ayuso --- extensions/libxt_string.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/extensions/libxt_string.c b/extensions/libxt_string.c index d298c6a7..7c6366cb 100644 --- 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; } -- cgit v1.2.3