json: reject trailing garbage

Yajl 2 has a nice feature that it can be configured whether to
allow multiple JSON objects parsed from a single stream, defaulting
to off.  And yajl 1.0.12 at least provided a way to tell if all
input bytes were parsed, or if trailing bytes remained after a
valid JSON object was parsed.  But we target RHEL 6 yajl 1.0.7,
which has neither of these.  So fake it by always parsing '[...]'
instead, so that trailing garbage either trips up the array parse,
or is easily detected when unwrapping the result.

* src/util/virjson.c (virJSONValueFromString): With older json,
wrap text to avoid trailing garbage.
* tests/jsontest.c (mymain): Add tests for this.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/util/virjson.c b/src/util/virjson.c
index 8d12fadb3f59637087522f30e194fb591992087e..a33005a7cfc6e98792ea7fa6365103e6dd4133bf 100644 (file)
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1597,6 +1597,7 @@ virJSONValueFromString(const char *jsonstring)
     size_t len = strlen(jsonstring);
 # ifndef WITH_YAJL2
     yajl_parser_config cfg = { 0, 1 };
+    virJSONValuePtr tmp;
 # endif
 
     VIR_DEBUG("string=%s", jsonstring);
@@ -1616,7 +1617,21 @@ virJSONValueFromString(const char *jsonstring)
         goto cleanup;
     }
 
+    /* Yajl 2 is nice enough to default to rejecting trailing garbage.
+     * Yajl 1.0.12 has yajl_get_bytes_consumed to make that detection
+     * simpler.  But we're stuck with yajl 1.0.7 on RHEL 6, which
+     * happily quits parsing at the end of a valid JSON construct,
+     * with no visibility into how much more input remains.  Wrapping
+     * things in an array forces yajl to confess the truth.  */
+# ifdef WITH_YAJL2
     rc = yajl_parse(hand, (const unsigned char *)jsonstring, len);
+# else
+    rc = yajl_parse(hand, (const unsigned char *)"[", 1);
+    if (VIR_YAJL_STATUS_OK(rc))
+        rc = yajl_parse(hand, (const unsigned char *)jsonstring, len);
+    if (VIR_YAJL_STATUS_OK(rc))
+        rc = yajl_parse(hand, (const unsigned char *)"]", 1);
+# endif
     if (!VIR_YAJL_STATUS_OK(rc) ||
         yajl_complete_parse(hand) != yajl_status_ok) {
         unsigned char *errstr = yajl_get_error(hand, 1,
@@ -1638,6 +1653,18 @@ virJSONValueFromString(const char *jsonstring)
         virJSONValueFree(parser.head);
     } else {
         ret = parser.head;
+# ifndef WITH_YAJL2
+        /* Undo the array wrapping above */
+        tmp = ret;
+        ret = NULL;
+        if (virJSONValueArraySize(tmp) > 1)
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot parse json %s: too many items present"),
+                           jsonstring);
+        else
+            ret = virJSONValueArraySteal(tmp, 0);
+        virJSONValueFree(tmp);
+# endif
     }
 
  cleanup:
@@ -1650,7 +1677,7 @@ virJSONValueFromString(const char *jsonstring)
         VIR_FREE(parser.state);
     }
 
-    VIR_DEBUG("result=%p", parser.head);
+    VIR_DEBUG("result=%p", ret);
 
     return ret;
 }

diff --git a/tests/jsontest.c b/tests/jsontest.c
index f6c2d84095d2206e111311e691f6ecaa72782da4..a363dc0d7ad9c48b7bca90cbac633d846b7bfbe6 100644 (file)
--- a/tests/jsontest.c
+++ b/tests/jsontest.c
@@ -419,6 +419,8 @@ mymain(void)
     DO_TEST_PARSE_FAIL("overdone keyword", "[ truest ]");
     DO_TEST_PARSE_FAIL("unknown keyword", "huh");
     DO_TEST_PARSE_FAIL("comments", "[ /* nope */\n1 // not this either\n]");
+    DO_TEST_PARSE_FAIL("trailing garbage", "[] []");
+    DO_TEST_PARSE_FAIL("list without array", "1, 1");
 
     DO_TEST_PARSE_FAIL("object with numeric keys", "{ 1:1, 2:1, 3:2 }");
     DO_TEST_PARSE_FAIL("unterminated object", "{ \"1\":1, \"2\":1, \"3\":2");