From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adUQh-00058z-7M for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:04:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adUQd-0002GC-Vm for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:04:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36843) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adUQd-0002FL-Mu for qemu-devel@nongnu.org; Tue, 08 Mar 2016 22:04:51 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 4E9CEC06C9D4 for ; Wed, 9 Mar 2016 03:04:51 +0000 (UTC) References: <1457420446-25276-1-git-send-email-peterx@redhat.com> <1457420446-25276-2-git-send-email-peterx@redhat.com> <87pov5e5w3.fsf@blackfin.pond.sub.org> <20160308101944.GD5807@noname.str.redhat.com> <20160309025738.GF2377@pxdev.xzpeter.org> From: Eric Blake Message-ID: <56DF92D2.9040905@redhat.com> Date: Tue, 8 Mar 2016 20:04:50 -0700 MIME-Version: 1.0 In-Reply-To: <20160309025738.GF2377@pxdev.xzpeter.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fGhkVac0vLx4NRcPmKvR1bwjQr3qH0WDQ" Subject: Re: [Qemu-devel] [PATCH 1/8] qdict: fix unbounded stack for qdict_array_entries List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu , Kevin Wolf Cc: qemu-devel@nongnu.org, pbonzini@redhat.com, Markus Armbruster , Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --fGhkVac0vLx4NRcPmKvR1bwjQr3qH0WDQ Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 03/08/2016 07:57 PM, Peter Xu wrote: > On Tue, Mar 08, 2016 at 11:19:44AM +0100, Kevin Wolf wrote: >> Am 08.03.2016 um 09:22 hat Markus Armbruster geschrieben: >>> Same arguments as for PATCH 2, except here an argument on the maximum= >>> length of subqdict would probably be easier. >> >> Yes, these are constant string literals in all callers, including the >> one non-test case in quorum. >> >> Let's simply assert a reasonable maximum for subqdict_length. The >> minimum we need to allow with the existing callers is 9, and I expect >> we'll never get keys longer than 16 characters. >=20 > Hi, Kevin, Markus, >=20 > The patch should be trying to do as mentioned above. To make it > clearer, how about the following one: >=20 > diff --git a/qobject/qdict.c b/qobject/qdict.c > index 9833bd0..dde99e0 100644 > --- a/qobject/qdict.c > +++ b/qobject/qdict.c > @@ -704,17 +704,16 @@ int qdict_array_entries(QDict *src, const char *s= ubqdict) > for (i =3D 0; i < INT_MAX; i++) { > QObject *subqobj; > int subqdict_entries; > - size_t slen =3D 32 + subqdict_len; > - char indexstr[slen], prefix[slen]; > + char indexstr[128], prefix[128]; > size_t snprintf_ret; >=20 > - snprintf_ret =3D snprintf(indexstr, slen, "%s%u", subqdict, i)= ; > - assert(snprintf_ret < slen); > + snprintf_ret =3D snprintf(indexstr, ARRAY_SIZE(indexstr), "%s%= u", subqdict, i); > + assert(snprintf_ret < ARRAY_SIZE(indexstr)); sizeof(indexstr) works, and is a bit nicer than ARRAY_SIZE() when dealing with char. But I'm worried that this can trigger an abort() by someone hammering on the command line. Just because we don't expect any QMP command to validate with a key name longer than 128 doesn't mean that we don't have to deal with a command line with a garbage key name that long. What's wrong with just using g_strdup_printf() and heap-allocating the result, avoiding snprintf() and fixed lengths altogether? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --fGhkVac0vLx4NRcPmKvR1bwjQr3qH0WDQ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJW35LSAAoJEKeha0olJ0NqWZEIAJoUoRxhgyk76CTASg5/8d6z d2pZrwX8pN94GbBxY3B70abfM7IZONJyfh7tWHc4O2MNnWE/aAzhWSGHIe8w25xf lGmhBhoClKn9SjY9NHi8FexM0Hnl9plq1Aidb7Vo2FR85GCd//iaZMKvY5LpUqYW ygV2jy0b1mwGBr4yYrl8nyJLcIRjvNRPsUGMlIJb5/4wlBDwUcjqUZFmwmtIrTOg fYMW+toBk9XMdIc1Szt7cVSQc3WeJOza9aFIShWhq/R4qTPC596p8OjEFBqrRxH3 buRzHDJKDS3AAUZ4LNy12iI4CpoW7zguaq9HI1pvy79i9WsZHbT/D5pq9PVregA= =O72J -----END PGP SIGNATURE----- --fGhkVac0vLx4NRcPmKvR1bwjQr3qH0WDQ--