From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>

Date: Mon, 30 Mar 2015 12:41:40 +0200

Subject: [PATCH] Strip control codes in virBufferEscapeString

These cannot be represented in XML.

We have been stripping them, but only if the string had

characters that needed escaping: <>"'&

Extend the strcspn check to include control codes, and strip

them even if we don't do any escaping.

https://bugzilla.redhat.com/show_bug.cgi?id=1184131

https://bugzilla.redhat.com/show_bug.cgi?id=1066564

(cherry picked from commit aeb5262e4397528d582682471cb8075141189465)

---

 src/util/virbuffer.c | 14 +++++++++++---

 tests/virbuftest.c   | 49 +++++++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c

index 706dbfa..3d13c90 100644

--- a/src/util/virbuffer.c

+++ b/src/util/virbuffer.c

@@ -438,6 +438,13 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)

     int len;

     char *escaped, *out;

     const char *cur;

+    const char forbidden_characters[] = {

+        0x01,   0x02,   0x03,   0x04,   0x05,   0x06,   0x07,   0x08,

+        /*\t*/  /*\n*/  0x0B,   0x0C,   /*\r*/  0x0E,   0x0F,   0x10,

+        0x11,   0x12,   0x13,   0x14,   0x15,   0x16,   0x17,   0x18,

+        0x19,   '"',    '&',    '\'',   '<',    '>',

+        '\0'

+    };

     if ((format == NULL) || (buf == NULL) || (str == NULL))

         return;

@@ -446,7 +453,7 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)

         return;

     len = strlen(str);

-    if (strcspn(str, "<>&'\"") == len) {

+    if (strcspn(str, forbidden_characters) == len) {

         virBufferAsprintf(buf, format, str);

         return;

     }

@@ -490,8 +497,7 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)

             *out++ = 'o';

             *out++ = 's';

             *out++ = ';';

-        } else if (((unsigned char)*cur >= 0x20) || (*cur == '\n') || (*cur == '\t') ||

-                   (*cur == '\r')) {

+        } else if (!strchr(forbidden_characters, *cur)) {

             /*

              * default case, just copy !

              * Note that character over 0x80 are likely to give problem

@@ -499,6 +505,8 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)

              * it's hard to handle properly we have to assume it's UTF-8 too

              */

             *out++ = *cur;

+        } else {

+            /* silently ignore control characters */

         }

         cur++;

     }

diff --git a/tests/virbuftest.c b/tests/virbuftest.c

index 21cb18b..10398d5 100644

--- a/tests/virbuftest.c

+++ b/tests/virbuftest.c

@@ -349,6 +349,39 @@ testBufAddStr(const void *opaque ATTRIBUTE_UNUSED)

 static int

+testBufEscapeStr(const void *opaque ATTRIBUTE_UNUSED)

+{

+    const struct testBufAddStrData *data = opaque;

+    virBuffer buf = VIR_BUFFER_INITIALIZER;

+    char *actual;

+    int ret = -1;

+

+    virBufferAddLit(&buf, "<c>\n");

+    virBufferAdjustIndent(&buf, 2);

+    virBufferEscapeString(&buf, "<el>%s</el>\n", data->data);

+    virBufferAdjustIndent(&buf, -2);

+    virBufferAddLit(&buf, "</c>");

+

+    if (!(actual = virBufferContentAndReset(&buf))) {

+        TEST_ERROR("buf is empty");

+        goto cleanup;

+    }

+

+    if (STRNEQ_NULLABLE(actual, data->expect)) {

+        TEST_ERROR("testBufEscapeStr(): Strings don't match:\n");

+        virtTestDifference(stderr, data->expect, actual);

+        goto cleanup;

+    }

+

+    ret = 0;

+

+ cleanup:

+    VIR_FREE(actual);

+    return ret;

+}

+

+

+static int

 mymain(void)

 {

     int ret = 0;

@@ -379,6 +412,22 @@ mymain(void)

     DO_TEST_ADD_STR("\n", "<c>\n  \n</c>");

     DO_TEST_ADD_STR("\n  \n\n", "<c>\n  \n    \n  \n</c>");

+#define DO_TEST_ESCAPE(data, expect)                                   \

+    do {                                                               \

+        struct testBufAddStrData info = { data, expect };              \

+        if (virtTestRun("Buf: EscapeStr", testBufEscapeStr, &info) < 0)   \

+            ret = -1;                                                  \

+    } while (0)

+

+    DO_TEST_ESCAPE("",

+                   "<c>\n  <el><td></td><td></td></el>\n</c>");

+    DO_TEST_ESCAPE("\007\"&&\"\x15",

+                   "<c>\n  <el>"&&"</el>\n</c>");

+    DO_TEST_ESCAPE(",,'..',,",

+                   "<c>\n  <el>,,'..',,</el>\n</c>");

+    DO_TEST_ESCAPE("\x01\x01\x02\x03\x05\x08",

+                   "<c>\n  <el></el>\n</c>");

+

     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;

 }