diff options
author | Markus Armbruster <armbru@redhat.com> | 2018-08-23 18:40:10 +0200 |
---|---|---|
committer | Markus Armbruster <armbru@redhat.com> | 2018-08-24 20:26:37 +0200 |
commit | 2a4794ba146d6560bd77ca840ff6908f81d585f4 (patch) | |
tree | 110f5c011d9a62f67a6acda473a7c087e147e6d3 /qobject/qjson.c | |
parent | 4d40066142278a7a9cfbde1ae481e0e94a4d24c3 (diff) | |
download | qemu-2a4794ba146d6560bd77ca840ff6908f81d585f4.zip qemu-2a4794ba146d6560bd77ca840ff6908f81d585f4.tar.gz qemu-2a4794ba146d6560bd77ca840ff6908f81d585f4.tar.bz2 |
qjson: Fix qobject_from_json() & friends for multiple values
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 <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-44-armbru@redhat.com>
Diffstat (limited to 'qobject/qjson.c')
-rw-r--r-- | qobject/qjson.c | 15 |
1 files changed, 14 insertions, 1 deletions
diff --git a/qobject/qjson.c b/qobject/qjson.c index 7395556..7f69036 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; } /* |