From nobody Wed Nov 5 10:30:11 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1535045411492631.2446139211579; Thu, 23 Aug 2018 10:30:11 -0700 (PDT) Received: from localhost ([::1]:37967 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fstQw-0002em-6n for importer@patchew.org; Thu, 23 Aug 2018 13:30:10 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fsskU-0000Xy-Ac for qemu-devel@nongnu.org; Thu, 23 Aug 2018 12:46:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fsskM-0006MY-8h for qemu-devel@nongnu.org; Thu, 23 Aug 2018 12:46:12 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58726 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fsskL-0006Lg-Rh for qemu-devel@nongnu.org; Thu, 23 Aug 2018 12:46:09 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7780240241C7; Thu, 23 Aug 2018 16:46:09 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-97.ams2.redhat.com [10.36.116.97]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 33DFCA9EF6; Thu, 23 Aug 2018 16:46:09 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id A34011169081; Thu, 23 Aug 2018 18:40:26 +0200 (CEST) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Thu, 23 Aug 2018 18:40:23 +0200 Message-Id: <20180823164025.12553-57-armbru@redhat.com> In-Reply-To: <20180823164025.12553-1-armbru@redhat.com> References: <20180823164025.12553-1-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Thu, 23 Aug 2018 16:46:09 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Thu, 23 Aug 2018 16:46:09 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'armbru@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH v3 56/58] json: Improve safety of qobject_from_jsonf_nofail() & friends X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: marcandre.lureau@redhat.com, mdroth@linux.vnet.ibm.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" The JSON parser optionally supports interpolation. This is used to build QObjects by parsing string templates. The templates are C literals, so parse errors (such as invalid interpolation specifications) are actually programming errors. Consequently, the functions providing parsing with interpolation (qobject_from_jsonf_nofail(), qobject_from_vjsonf_nofail(), qdict_from_jsonf_nofail(), qdict_from_vjsonf_nofail()) pass &error_abort to the parser. However, there's another, more dangerous kind of programming error: since we use va_arg() to get the value to interpolate, behavior is undefined when the variable argument isn't consistent with the interpolation specification. The same problem exists with printf()-like functions, and the solution is to have the compiler check consistency. This is what GCC_FMT_ATTR() is about. To enable this type checking for interpolation as well, we carefully chose our interpolation specifications to match printf conversion specifications, and decorate functions parsing templates with GCC_FMT_ATTR(). Note that this only protects against undefined behavior due to type errors. It can't protect against use of invalid interpolation specifications that happen to be valid printf conversion specifications. However, there's still a gaping hole in the type checking: GCC recognizes '%' as start of printf conversion specification anywhere in the template, but the parser recognizes it only outside JSON strings. For instance, if someone were to pass a "{ '%s': %d }" template, GCC would require a char * and an int argument, but the parser would va_arg() only an int argument, resulting in undefined behavior. Avoid undefined behavior by catching the programming error at run time: have the parser recognize and reject '%' in JSON strings. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake --- qobject/json-parser.c | 12 ++++++++++-- tests/check-qjson.c | 17 +++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/qobject/json-parser.c b/qobject/json-parser.c index 273f448a52..63e9229f1c 100644 --- a/qobject/json-parser.c +++ b/qobject/json-parser.c @@ -144,7 +144,8 @@ static QString *parse_string(JSONParserContext *ctxt, J= SONToken *token) =20 while (*ptr !=3D quote) { assert(*ptr); - if (*ptr =3D=3D '\\') { + switch (*ptr) { + case '\\': beg =3D ptr++; switch (*ptr++) { case '"': @@ -205,7 +206,14 @@ static QString *parse_string(JSONParserContext *ctxt, = JSONToken *token) parse_error(ctxt, token, "invalid escape sequence in strin= g"); goto out; } - } else { + break; + case '%': + if (ctxt->ap) { + parse_error(ctxt, token, "can't interpolate into string"); + goto out; + } + /* fall through */ + default: cp =3D mod_utf8_codepoint(ptr, 6, &end); if (cp < 0) { parse_error(ctxt, token, "invalid UTF-8 sequence in string= "); diff --git a/tests/check-qjson.c b/tests/check-qjson.c index 936258ddd4..a1854573de 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -1037,16 +1037,13 @@ static void interpolation_unknown(void) =20 static void interpolation_string(void) { - QLitObject decoded =3D QLIT_QLIST(((QLitObject[]){ - QLIT_QSTR("%s"), - QLIT_QSTR("eins"), - {}})); - QObject *qobj; - - /* Dangerous misfeature: % is silently ignored in strings */ - qobj =3D qobject_from_jsonf_nofail("['%s', %s]", "eins", "zwei"); - g_assert(qlit_equal_qobject(&decoded, qobj)); - qobject_unref(qobj); + if (g_test_subprocess()) { + qobject_from_jsonf_nofail("['%s', %s]", "eins", "zwei"); + } + g_test_trap_subprocess(NULL, 0, 0); + g_test_trap_assert_failed(); + g_test_trap_assert_stderr("*Unexpected error*" + "can't interpolate into string*"); } =20 static void simple_dict(void) --=20 2.17.1