[pve-devel] applied: [PATCH pve-libspice-server] Add fix for CVE-2019-3813
Wolfgang Bumiller
w.bumiller at proxmox.com
Tue Jan 29 12:17:20 CET 2019
applied
On Tue, Jan 29, 2019 at 10:21:33AM +0100, Dominik Csapak wrote:
> Fix comes from oss-security at lists.openwall.com
> changed g_critical to spice_critical so that the patch applies
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> Makefile | 2 +-
> debian/changelog | 6 ++
> ...-off-by-one-error-in-group-slot-boundary-.patch | 102 +++++++++++++++++++++
> debian/patches/series | 1 +
> 4 files changed, 110 insertions(+), 1 deletion(-)
> create mode 100644 debian/patches/0001-memslot-Fix-off-by-one-error-in-group-slot-boundary-.patch
>
> diff --git a/Makefile b/Makefile
> index 54f2bfa..92b953d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,7 @@ RELEASE=4.0
>
> PACKAGE=pve-libspice-server1
> PKGVERSION=0.14.1
> -PKGRELEASE=1
> +PKGRELEASE=2
>
> PKGDIR=spice-${PKGVERSION}
> PKGSRC=${PKGDIR}.tar.bz2
> diff --git a/debian/changelog b/debian/changelog
> index a987473..bbb370b 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,9 @@
> +pve-libspice-server (0.14.1-2) unstable; urgency=medium
> +
> + * Fix CVE-2019-3813
> +
> + -- Promxox Support Team <support at proxmox.com> Tue, 29 Jan 2019 09:46:41 +0100
> +
> pve-libspice-server (0.14.1-1) unstable; urgency=medium
>
> * upgrade to 0.14.1
> diff --git a/debian/patches/0001-memslot-Fix-off-by-one-error-in-group-slot-boundary-.patch b/debian/patches/0001-memslot-Fix-off-by-one-error-in-group-slot-boundary-.patch
> new file mode 100644
> index 0000000..30aed66
> --- /dev/null
> +++ b/debian/patches/0001-memslot-Fix-off-by-one-error-in-group-slot-boundary-.patch
> @@ -0,0 +1,102 @@
> +From 6eff47e72cb2f23d168be58bab8bdd60df49afd0 Mon Sep 17 00:00:00 2001
> +From: Christophe Fergeau <cfergeau at redhat.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 at redhat.com>
> +Signed-off-by: Dominik Csapak <d.csapak at proxmox.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) {
> + spice_critical("group_id too big");
> + return NULL;
> + }
> +
> + slot_id = memslot_get_id(info, addr);
> +- if (slot_id > info->num_memslots) {
> ++ if (slot_id >= info->num_memslots) {
> + print_memslots(info);
> + spice_critical("slot_id %d too big, addr=%" PRIx64, slot_id, addr);
> + return NULL;
> +diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
> +index 8565239f0..447425984 100644
> +--- a/server/tests/test-qxl-parsing.c
> ++++ b/server/tests/test-qxl-parsing.c
> +@@ -98,6 +98,31 @@ static void deinit_qxl_surface(QXLSurfaceCmd *qxl)
> + g_free(from_physical(qxl->u.surface_create.data));
> + }
> +
> ++static void test_memslot_invalid_group_id(void)
> ++{
> ++ RedMemSlotInfo mem_info;
> ++ init_meminfo(&mem_info);
> ++
> ++ memslot_get_virt(&mem_info, 0, 16, 1);
> ++}
> ++
> ++static void test_memslot_invalid_slot_id(void)
> ++{
> ++ RedMemSlotInfo mem_info;
> ++ init_meminfo(&mem_info);
> ++
> ++ memslot_get_virt(&mem_info, 1 << mem_info.memslot_id_shift, 16, 0);
> ++}
> ++
> ++static void test_memslot_invalid_addresses(void)
> ++{
> ++ g_test_trap_subprocess("/server/memslot-invalid-addresses/subprocess/group_id", 0, 0);
> ++ g_test_trap_assert_stderr("*group_id too big*");
> ++
> ++ g_test_trap_subprocess("/server/memslot-invalid-addresses/subprocess/slot_id", 0, 0);
> ++ g_test_trap_assert_stderr("*slot_id 1 too big*");
> ++}
> ++
> + static void test_no_issues(void)
> + {
> + RedMemSlotInfo mem_info;
> +@@ -317,6 +342,11 @@ int main(int argc, char *argv[])
> + {
> + g_test_init(&argc, &argv, NULL);
> +
> ++ /* try to use invalid memslot group/slot */
> ++ g_test_add_func("/server/memslot-invalid-addresses", test_memslot_invalid_addresses);
> ++ g_test_add_func("/server/memslot-invalid-addresses/subprocess/group_id", test_memslot_invalid_group_id);
> ++ g_test_add_func("/server/memslot-invalid-addresses/subprocess/slot_id", test_memslot_invalid_slot_id);
> ++
> + /* try to create a surface with no issues, should succeed */
> + g_test_add_func("/server/qxl-parsing-no-issues", test_no_issues);
> +
> +--
> +2.19.2
> +
> diff --git a/debian/patches/series b/debian/patches/series
> index 247f05d..ab02eda 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -1 +1,2 @@
> allow-to-set-sasl-callbacks.patch
> +0001-memslot-Fix-off-by-one-error-in-group-slot-boundary-.patch
> --
> 2.11.0
More information about the pve-devel
mailing list