From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59967)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1WVlPq-0001WK-3k
	for qemu-devel@nongnu.org; Thu, 03 Apr 2014 13:27:08 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1WVlPi-0001b7-Bh
	for qemu-devel@nongnu.org; Thu, 03 Apr 2014 13:27:01 -0400
Received: from mail-lb0-f169.google.com ([209.85.217.169]:52339)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1WVlPi-0001ar-2g
	for qemu-devel@nongnu.org; Thu, 03 Apr 2014 13:26:54 -0400
Received: by mail-lb0-f169.google.com with SMTP id q8so1625778lbi.28
	for <qemu-devel@nongnu.org>; Thu, 03 Apr 2014 10:26:52 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <1396543778-22307-6-git-send-email-mst@redhat.com>
References: <1396543778-22307-1-git-send-email-mst@redhat.com>
	<1396543778-22307-6-git-send-email-mst@redhat.com>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Thu, 3 Apr 2014 18:26:32 +0100
Message-ID: <CAFEAcA-3PP=-gfxXh-aEHi7=2BdfZ-RkFnYyUD9v2TKoVvKo5Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH v5 05/24] virtio-net: out-of-bounds buffer
	write on load
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>, qemu-stable <qemu-stable@nongnu.org>, QEMU Developers <qemu-devel@nongnu.org>, Anthony Liguori <aliguori@amazon.com>, Dave Gilbert <dgilbert@redhat.com>

On 3 April 2014 17:50, Michael S. Tsirkin <mst@redhat.com> wrote:
> CVE-2013-4149 QEMU 1.3.0 out-of-bounds buffer write in
> virtio_net_load()@hw/net/virtio-net.c
>
>>         } else if (n->mac_table.in_use) {
>>             uint8_t *buf = g_malloc0(n->mac_table.in_use);
>
> We are allocating buffer of size n->mac_table.in_use
>
>>             qemu_get_buffer(f, buf, n->mac_table.in_use * ETH_ALEN);
>
> and read to the n->mac_table.in_use size buffer n->mac_table.in_use *
> ETH_ALEN bytes, corrupting memory.
>
> If adversary controls state then memory written there is controlled
> by adversary.
>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/net/virtio-net.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 439477b..c247529 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1362,10 +1362,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
>          if (n->mac_table.in_use <= MAC_TABLE_ENTRIES) {
>              qemu_get_buffer(f, n->mac_table.macs,
>                              n->mac_table.in_use * ETH_ALEN);
> -        } else if (n->mac_table.in_use) {
> -            uint8_t *buf = g_malloc0(n->mac_table.in_use);
> -            qemu_get_buffer(f, buf, n->mac_table.in_use * ETH_ALEN);
> -            g_free(buf);
> +        } else {
> +            int i;
> +
> +            /* Overflow detected - can happen if source has a larger MAC table.
> +             * We simply set overflow flag so there's no need to maintain the
> +             * table of addresses, discard them all.
> +             */
> +            for (i = 0; i < n->mac_table.in_use * ETH_ALEN; ++i) {
> +                qemu_get_byte(f);
> +            }

If the incoming data sets the in_use field to INT_MAX then
we get integer overflow on the multiply here. That's
undefined behaviour in C and we probably shouldn't allow
that to happen.

thanks
-- PMM