[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 43/60] qjson: Fix qobject_from_json() & friends f
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PATCH v2 43/60] qjson: Fix qobject_from_json() & friends for multiple values |
Date: |
Fri, 17 Aug 2018 17:05:42 +0200 |
qobject_from_json() & friends use the consume_json() callback to
receive either a value or an error from the parser.
When they are fed a string that contains more than either one JSON
value or one JSON syntax error, consume_json() gets called multiple
times.
When the last call receives a value, qobject_from_json() returns that
value. Any other values are leaked.
When any call receives an error, qobject_from_json() sets the first
error received. Any other errors are thrown away.
When values follow errors, qobject_from_json() returns both a value
and sets an error. That's bad. Impact:
* block.c's parse_json_protocol() ignores and leaks the value. It's
used to to parse pseudo-filenames starting with "json:". The
pseudo-filenames can come from the user or from image meta-data such
as a QCOW2 image's backing file name.
* vl.c's parse_display_qapi() ignores and leaks the error. It's used
to parse the argument of command line option -display.
* vl.c's main() case QEMU_OPTION_blockdev ignores the error and leaves
it in @err. main() will then pass a pointer to a non-null Error *
to net_init_clients(), which is forbidden. It can lead to assertion
failure or other misbehavior.
* check-qjson.c's multiple_values() demonstrates the badness.
* The other callers are not affected since they only pass strings with
exactly one JSON value or, in the case of negative tests, one
error.
The impact on the _nofail() functions is relatively harmless. They
abort when any call receives an error. Else they return the last
value, and leak the others, if any.
Fix consume_json() as follows. On the first call, save value and
error as before. On subsequent calls, if any, don't save them. If
the first call saved a value, the next call, if any, replaces the
value by an "Expecting at most one JSON value" error. Take care not
to leak values or errors that aren't saved.
Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
---
qobject/qjson.c | 15 ++++++++++++++-
tests/check-qjson.c | 10 +++-------
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 7395556069..7f69036487 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -33,8 +33,21 @@ static void consume_json(void *opaque, QObject *json, Error
*err)
{
JSONParsingState *s = opaque;
+ assert(!json != !err);
+ assert(!s->result || !s->err);
+
+ if (s->result) {
+ qobject_unref(s->result);
+ s->result = NULL;
+ error_setg(&s->err, "Expecting at most one JSON value");
+ }
+ if (s->err) {
+ qobject_unref(json);
+ error_free(err);
+ return;
+ }
s->result = json;
- error_propagate(&s->err, err);
+ s->err = err;
}
/*
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index d741d29733..22a3225358 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1443,17 +1443,13 @@ static void multiple_values(void)
Error *err = NULL;
QObject *obj;
- /* BUG this leaks the syntax tree for "false" */
obj = qobject_from_json("false true", &err);
- g_assert(qbool_get_bool(qobject_to(QBool, obj)));
- g_assert(!err);
- qobject_unref(obj);
+ error_free_or_abort(&err);
+ g_assert(obj == NULL);
- /* BUG simultaneously succeeds and fails */
obj = qobject_from_json("} true", &err);
- g_assert(qbool_get_bool(qobject_to(QBool, obj)));
error_free_or_abort(&err);
- qobject_unref(obj);
+ g_assert(obj == NULL);
}
int main(int argc, char **argv)
--
2.17.1
- [Qemu-devel] [PATCH v2 59/60] json: Improve safety of qobject_from_jsonf_nofail() & friends, (continued)
- [Qemu-devel] [PATCH v2 59/60] json: Improve safety of qobject_from_jsonf_nofail() & friends, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 46/60] json: Assert json_parser_parse() consumes all tokens on success, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 45/60] json: Fix streamer not to ignore trailing unterminated structures, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 41/60] json: Nicer recovery from invalid leading zero, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 40/60] json: Replace %I64d, %I64u by %PRId64, %PRIu64, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 49/60] json: Streamline json_message_process_token(), Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 39/60] json: Leave rejecting invalid interpolation to parser, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 51/60] json: Eliminate lexer state IN_ERROR and pseudo-token JSON_MIN, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 43/60] qjson: Fix qobject_from_json() & friends for multiple values,
Markus Armbruster <=
- [Qemu-devel] [PATCH v2 57/60] tests/drive_del-test: Fix harmless JSON interpolation bug, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 48/60] json: Enforce token count and size limits more tightly, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 56/60] docs/interop/qmp-spec: How to force known good parser state, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 54/60] qobject: Drop superfluous includes of qemu-common.h, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 47/60] qjson: Have qobject_from_json() & friends reject empty and blank, Markus Armbruster, 2018/08/17
- [Qemu-devel] [PATCH v2 55/60] json: Clean up headers, Markus Armbruster, 2018/08/17
- Re: [Qemu-devel] [PATCH v2 00/60] json: Fixes, error reporting improvements, cleanups, no-reply, 2018/08/18