[Xen-devel] [PATCH 6/6] xl: Rewrite trim()

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

This function would produce a NULL output pointer if the input was an
empty string, leading to a crash.

I don't think this is likely to be a security problem, as the two call
sites involve configuration options which callers are unlikely to
expose to other-than-fully-trusted input.

Also, the function would needlessly copy the input string (which I
care about not for performance reasons but because it makes the memory
handling more confusing), and would mishandle strings which contained
only predicate-true characters.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 tools/libxl/xl_cmdimpl.c |   35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4396095..1966316 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -647,26 +647,23 @@ typedef int (*char_predicate_t)(const int c);
 
 static void trim(char_predicate_t predicate, const char *input, char **output)
 {
-    char *p, *q, *tmp;
+    const char *first, *after;
 
-    *output = NULL;
-    if (*input == '\000')
-        return;
-    /* Input has length >= 1 */
-
-    p = tmp = xstrdup(input);
-    /* Skip past the characters for which predicate is true */
-    while ((*p != '\000') && (predicate((unsigned char)*p)))
-        p ++;
-    q = p + strlen(p) - 1;
-    /* q points to the last non-NULL character */
-    while ((q > p) && (predicate((unsigned char)*q)))
-        q --;
-    /* q points to the last character we want */
-    q ++;
-    *q = '\000';
-    *output = xstrdup(p);
-    free(tmp);
+    for (first = input;
+         *first && predicate((unsigned char)first[0]);
+         first++)
+        ;
+
+    for (after = first + strlen(first);
+         after > first && predicate((unsigned char)after[-1]);
+         after--)
+        ;
+
+    size_t len_nonnull = after - first;
+
+    *output = xmalloc(len_nonnull + 1);
+    memcpy(output, first, len_nonnull);
+    output[len_nonnull] = 0;
 }
 
 static int split_string_into_pair(const char *str,
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel