[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 10/11] json-streamer: limit the maximum recursio

From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 10/11] json-streamer: limit the maximum recursion depth and maximum token count
Date: Sat, 12 Mar 2011 09:03:52 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8

On 03/11/2011 05:16 PM, Michael Roth wrote:
          parser->emit(parser, parser->tokens);
          parser->tokens = qlist_new();
+        parser->token_size = 0;
+    } else if (parser->token_size>  MAX_TOKEN_SIZE ||
+               parser->bracket_count>  MAX_NESTING ||
+               parser->brace_count>  MAX_NESTING) {
+ /* Security consideration, we limit total memory allocated per object
+         * and the maximum recursion depth that a message can force.
+         */
+        parser->brace_count = 0;
+        parser->bracket_count = 0;
+        parser->emit(parser, parser->tokens);
@@ -56,6 +61,19 @@ static void json_message_process_token(JSONLexer *lexer, QString *token, JSONTok

Should we even bother passing this to the parser? That's a lot stuff to churn on that we know isn't going to result in anything useful. If there was a proper object earlier in the stream it would've been emitted when brace/bracket count reached 0.

I think it might be nicer to do parser->emit(parser, NULL), and fix up json_parser_parse_err() to check for this and simply return NULL back to qmp_dispatch_err() or whoever is calling.

I think brace_count < 0 || bracket_count < 0 should get similar treatment.

I think the main advantage of doing it this way is that we can test the maximum stack usage by just doing a simple:

(while true; do echo "{'key': "; done) | socat stdio unix:/path/to/qmp.sock > /dev/null

However, if we don't pass the token list to the parser, we need to know exactly what the maximum is and only generate that depth in order to test stack usage.

The parser needs to be robust against bad input so from a test perspective, I like the idea of passing something to the parser even if we know it's bad.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]