From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51735 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzDO8-0005t3-W4 for qemu-devel@nongnu.org; Mon, 14 Mar 2011 15:25:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzDO7-0004MH-KR for qemu-devel@nongnu.org; Mon, 14 Mar 2011 15:25:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53586) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzDO7-0004M3-9k for qemu-devel@nongnu.org; Mon, 14 Mar 2011 15:25:07 -0400 Date: Mon, 14 Mar 2011 16:25:02 -0300 From: Luiz Capitulino Message-ID: <20110314162502.12a7deab@doriath> In-Reply-To: <1299877249-13433-10-git-send-email-aliguori@us.ibm.com> References: <1299877249-13433-1-git-send-email-aliguori@us.ibm.com> <1299877249-13433-10-git-send-email-aliguori@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 09/11] json-lexer: limit the maximum size of a given token List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Paolo Bonzini , Michael@gnu.org, qemu-devel@nongnu.org, Roth , Markus Armbruster On Fri, 11 Mar 2011 15:00:47 -0600 Anthony Liguori wrote: > This is a security consideration. We don't want a client to cause an arbitrary > amount of memory to be allocated in QEMU. For now, we use a limit of 64MB > which should be large enough for any reasonably sized token. > > This is important for parsing JSON from untrusted sources. > > Signed-off-by: Anthony Liguori > > diff --git a/json-lexer.c b/json-lexer.c > index 834d7af..3462c89 100644 > --- a/json-lexer.c > +++ b/json-lexer.c > @@ -18,6 +18,8 @@ > #include "qemu-common.h" > #include "json-lexer.h" > > +#define MAX_TOKEN_SIZE (64ULL << 20) > + > /* > * \"([^\\\"]|(\\\"\\'\\\\\\/\\b\\f\\n\\r\\t\\u[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]))*\" > * '([^\\']|(\\\"\\'\\\\\\/\\b\\f\\n\\r\\t\\u[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]))*' > @@ -312,6 +314,17 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch) > } > lexer->state = new_state; > } while (!char_consumed); > + > + /* Do not let a single token grow to an arbitrarily large size, > + * this is a security consideration. > + */ > + if (lexer->token->length > MAX_TOKEN_SIZE) { > + lexer->emit(lexer, lexer->token, lexer->state, lexer->x, lexer->y); > + QDECREF(lexer->token); > + lexer->token = qstring_new(); > + lexer->state = IN_START; > + } Entering an invalid token is an error, we should fail here. Which brings two features: 1. A test code could trigger this condition and check for the specific error code 2. Developers will know when they hit the limit. Although I don't expect expect this to happen, there was talking about adding base64 support to transfer something (I can't remember what, but we never know how the protocol will evolve). Also, by testing this I found that the parser seems to get confused when the limit is reached: it stops responding. > + > return 0; > } >