[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Mon, 28 Jan 2019 23:25:06 +0100
From: Peter Korsgaard <peter@...sgaard.com>
To: Scott Gayou <sgayou@...hat.com>
Cc: oss-security@...ts.openwall.com
Subject: Re: CVE-2019-3813: spice: Off-by-one error in array access in spice/server/memslot.c
>>>>> "Scott" == Scott Gayou <sgayou@...hat.com> writes:
> Hello,
> spice versions 0.5.2 through 0.14.1 are vulnerable to an out-of-bounds read
> due to an off-by-one error in memslot_get_virt. This may lead to a
> denial-of-service, or, in the worst case, code-execution by unauthenticated
> attackers.
> The attached patch fixes the issue in spice and is planned to be included
> in forthcoming release spice 0.14.2.
> This issue was reported by Christophe Fergeau (Red Hat).
> References:
> https://bugzilla.redhat.com/show_bug.cgi?id=1665371
> Thank you.
> --
> Scott Gayou / Red Had Product Security
> From 6eff47e72cb2f23d168be58bab8bdd60df49afd0 Mon Sep 17 00:00:00 2001
> From: Christophe Fergeau <cfergeau@...hat.com>
> Date: Thu, 29 Nov 2018 14:18:39 +0100
> Subject: [spice-server] memslot: Fix off-by-one error in group/slot boundary
> check
> RedMemSlotInfo keeps an array of groups, and each group contains an
> array of slots. Unfortunately, these checks are off by 1, they check
> that the index is greater or equal to the number of elements in the
> array, while these arrays are 0 based. The check should only check for
> strictly greater than the number of elements.
> For the group array, this is not a big issue, as these memslot groups
> are created by spice-server users (eg QEMU), and the group ids used to
> index that array are also generated by the spice-server user, so it
> should not be possible for the guest to set them to arbitrary values.
> The slot id is more problematic, as it's calculated from a QXLPHYSICAL
> address, and such addresses are usually set by the guest QXL driver, so
> the guest can set these to arbitrary values, including malicious values,
> which are probably easy to build from the guest PCI configuration.
> This patch fixes the arrays bound check, and adds a test case for this.
> Signed-off-by: Christophe Fergeau <cfergeau@...hat.com>
> ---
> server/memslot.c | 4 ++--
> server/tests/test-qxl-parsing.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 2 deletions(-)
> diff --git a/server/memslot.c b/server/memslot.c
> index b27324efb..fb3d5cfd5 100644
> --- a/server/memslot.c
> +++ b/server/memslot.c
> @@ -97,13 +97,13 @@ void *memslot_get_virt(RedMemSlotInfo *info, QXLPHYSICAL addr, uint32_t add_size
> MemSlot *slot;
> - if (group_id > info->num_memslots_groups) {
> + if (group_id >= info->num_memslots_groups) {
> g_critical("group_id too big");
What version is this patch against? I don't see memslot.c using
g_critical() neither on the 0.14 branch (which doesn't have 0.14.1) or
master?
https://gitlab.freedesktop.org/spice/spice/blob/master/server/memslot.c#L97
--
Bye, Peter Korsgaard
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
