>From afc52674a8fd5002e6ce2c9e8177142c01128cbc Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 20 Jun 2014 17:04:59 +0200 Subject: [PATCH] UTIL: Fix access out of bound in parse_args While parsing string with multiple whitespaces, it may happen variable i is zero and we want to test end of argument "tmp[i-1] != '\0'". Side effect of this bug is duplicite string output array. Input string: "foo b" Expected output: { "foo", "a", NULL } Output: { "foo", "foo", "a", NULL } This patch uses inverted logic. Instead of testing whether to read next char or skip multiple whitespaces, we will test whether we have new argument which should be stored in output array. --- src/tests/util-tests.c | 30 ++++++++++++++++++++++++++---- src/util/util.c | 26 +++++++++++++------------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c index 198edf597b1bc8f1133e5c934e7f6fa2f6d4e021..b79153e3e89b90182bfce7c504a7d6a764a0e59b 100644 --- a/src/tests/util-tests.c +++ b/src/tests/util-tests.c @@ -141,6 +141,9 @@ START_TEST(test_parse_args) const char *parsed6[] = { "foo", "a", "e", NULL }; const char *parsed7[] = { "foo", "a", "f", NULL }; const char *parsed8[] = { "foo", "a\tg", NULL }; + const char *parsed9[] = { "foo", NULL }; + const char *parsed10[] = { " ", "foo", "\t", "\\'", NULL }; + const char *parsed11[] = { "a", NULL }; struct pa_testcase tc[] = { { "foo", parsed1 }, { "foo a", parsed2 }, @@ -148,15 +151,22 @@ START_TEST(test_parse_args) { "foo a\\ c", parsed4 }, { "foo a d ", parsed5 }, { "foo a e ", parsed6 }, - { "foo a f ", parsed7 }, + { "foo\ta\t \tf \t", parsed7 }, { "foo a\\\tg", parsed8 }, + { " foo ", parsed9 }, + { "\\ foo \\\t \\' ", parsed10 }, + { "a", parsed11 }, + { " ", NULL }, + { "", NULL }, + { " \t ", NULL }, { NULL, NULL } }; for (i=0; tc[i].argstr != NULL; i++) { parsed = parse_args(tc[i].argstr); fail_if(parsed == NULL && tc[i].parsed != NULL, - "Could not parse correct argument string '%s'\n"); + "Could not parse correct %d argument string '%s'\n", + i, tc[i].argstr); ret = diff_string_lists(test_ctx, parsed, discard_const(tc[i].parsed), &only_ret, &only_exp, &both); @@ -164,8 +174,20 @@ START_TEST(test_parse_args) fail_unless(only_ret[0] == NULL, "The parser returned more data than expected\n"); fail_unless(only_exp[0] == NULL, "The parser returned less data than expected\n"); - for (ii = 0; parsed[ii]; ii++) free(parsed[ii]); - free(parsed); + if (parsed) { + int parsed_len; + int expected_len; + + for (parsed_len=0; parsed[parsed_len]; ++parsed_len); + for (expected_len=0; tc[i].parsed[expected_len]; ++expected_len); + + fail_unless(parsed_len == expected_len, + "Test %d: length of 1st array [%d] != length of 2nd " + "array[%d]\n", i, parsed_len, expected_len); + + for (ii = 0; parsed[ii]; ii++) free(parsed[ii]); + free(parsed); + } } talloc_free(test_ctx); diff --git a/src/util/util.c b/src/util/util.c index ad93ca1a0715e3a5f5c6c7371eb142718c38afa3..7f80771ecd9868feaf43e34cbd61e44dd8ae5f3a 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -153,7 +153,8 @@ char **parse_args(const char *str) num = 0; i = 0; e = false; - w = false; + /* skip leading whitespaces */ + w = true; p = str; while (*p) { if (*p == '\\') { @@ -205,19 +206,18 @@ char **parse_args(const char *str) tmp[i] = '\0'; i++; } - if (tmp[i-1] != '\0' || strlen(tmp) == 0) { - /* check next char and skip multiple spaces */ - continue; - } - r = realloc(ret, (num + 2) * sizeof(char *)); - if (!r) goto fail; - ret = r; - ret[num+1] = NULL; - ret[num] = strdup(tmp); - if (!ret[num]) goto fail; - num++; - i = 0; + /* save token to result array */ + if (i > 1 && tmp[i-1] == '\0') { + r = realloc(ret, (num + 2) * sizeof(char *)); + if (!r) goto fail; + ret = r; + ret[num+1] = NULL; + ret[num] = strdup(tmp); + if (!ret[num]) goto fail; + num++; + i = 0; + } } free(tmp); -- 1.9.3