From 852722ecb5dc09fc80cd3c837edb1cf6db529210 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn@redhat.com>
Date: Fri, 20 Jun 2014 17:04:59 +0200
Subject: 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.

Reviewed-by: Pavel Reichl <preichl@redhat.com>
Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
---
 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 198edf59..b79153e3 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 ad93ca1a..7f80771e 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);
-- 
cgit