From 84a56f38b23440cb3127eaffe4e495826a29f18c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 23 Aug 2018 18:40:06 +0200 Subject: json: Pass lexical errors and limit violations to callback The callback to consume JSON values takes QObject *json, Error *err. If both are null, the callback is supposed to make up an error by itself. This sucks. qjson.c's consume_json() neglects to do so, which makes qobject_from_json() null instead of failing. I consider that a bug. The culprit is json_message_process_token(): it passes two null pointers when it runs into a lexical error or a limit violation. Fix it to pass a proper Error object then. Update the callbacks: * monitor.c's handle_qmp_command(): the code to make up an error is now dead, drop it. * qga/main.c's process_event(): lumps the "both null" case together with the "not a JSON object" case. The former is now gone. The error message "Invalid JSON syntax" is misleading for the latter. Improve it to "Input must be a JSON object". * qobject/qjson.c's consume_json(): no update; check-qjson demonstrates qobject_from_json() now sets an error on lexical errors, but still doesn't on some other errors. * tests/libqtest.c's qmp_response(): the Error object is now reliable, so use it to improve the error message. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20180823164025.12553-40-armbru@redhat.com> --- qobject/json-lexer.c | 3 +-- qobject/json-streamer.c | 22 ++++++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) (limited to 'qobject') diff --git a/qobject/json-lexer.c b/qobject/json-lexer.c index 96fe13621d..7c31c2c8ff 100644 --- a/qobject/json-lexer.c +++ b/qobject/json-lexer.c @@ -334,8 +334,7 @@ static void json_lexer_feed_char(JSONLexer *lexer, char ch, bool flush) /* XXX: To avoid having previous bad input leaving the parser in an * unresponsive state where we consume unpredictable amounts of * subsequent "good" input, percolate this error state up to the - * tokenizer/parser by forcing a NULL object to be emitted, then - * reset state. + * parser by emitting a JSON_ERROR token, then reset lexer state. * * Also note that this handling is required for reliable channel * negotiation between QMP and the guest agent, since chr(0xFF) diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c index a373e0114a..e372ecc895 100644 --- a/qobject/json-streamer.c +++ b/qobject/json-streamer.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" +#include "qapi/error.h" #include "qapi/qmp/json-lexer.h" #include "qapi/qmp/json-parser.h" #include "qapi/qmp/json-streamer.h" @@ -57,6 +58,7 @@ void json_message_process_token(JSONLexer *lexer, GString *input, parser->bracket_count--; break; case JSON_ERROR: + error_setg(&err, "JSON parse error, stray '%s'", input->str); goto out_emit; default: break; @@ -82,12 +84,20 @@ void json_message_process_token(JSONLexer *lexer, GString *input, goto out_emit; } - if (parser->token_size > MAX_TOKEN_SIZE || - g_queue_get_length(parser->tokens) > MAX_TOKEN_COUNT || - parser->bracket_count + parser->brace_count > MAX_NESTING) { - /* Security consideration, we limit total memory allocated per object - * and the maximum recursion depth that a message can force. - */ + /* + * Security consideration, we limit total memory allocated per object + * and the maximum recursion depth that a message can force. + */ + if (parser->token_size > MAX_TOKEN_SIZE) { + error_setg(&err, "JSON token size limit exceeded"); + goto out_emit; + } + if (g_queue_get_length(parser->tokens) > MAX_TOKEN_COUNT) { + error_setg(&err, "JSON token count limit exceeded"); + goto out_emit; + } + if (parser->bracket_count + parser->brace_count > MAX_NESTING) { + error_setg(&err, "JSON nesting depth limit exceeded"); goto out_emit; } -- cgit v1.2.3