Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app-emulation/libvirt: Backport fixes for two CVEs #36242

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
From 2127032ed8cd49001465dc0dce9f842e13467bc2 Mon Sep 17 00:00:00 2001
Message-ID: <2127032ed8cd49001465dc0dce9f842e13467bc2.1713033988.git.mprivozn@redhat.com>
From: Martin Kletzander <mkletzan@redhat.com>
Date: Tue, 27 Feb 2024 16:20:12 +0100
Subject: [PATCH 1/2] Fix off-by-one error in udevListInterfacesByStatus
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ever since this function was introduced in 2012 it could've tried
filling in an extra interface name. That was made worse in 2019 when
the caller functions started accepting NULL arrays of size 0.

This is assigned CVE-2024-1441.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reported-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Fixes: 5a33366f5c0b18c93d161bd144f9f079de4ac8ca
Fixes: d6064e2759a24e0802f363e3a810dc5a7d7ebb15
Reviewed-by: Ján Tomko <jtomko@redhat.com>
(cherry picked from commit c664015fe3a7bf59db26686e9ed69af011c6ebb8)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/interface/interface_backend_udev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index fb6799ed94..4091483060 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -222,7 +222,7 @@ udevListInterfacesByStatus(virConnectPtr conn,
g_autoptr(virInterfaceDef) def = NULL;

/* Ensure we won't exceed the size of our array */
- if (count > names_len)
+ if (count >= names_len)
break;

path = udev_list_entry_get_name(dev_entry);
--
2.43.2

Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
From 10fa5f6ba64b354b99b0f7b372e66e45bb4d9379 Mon Sep 17 00:00:00 2001
Message-ID: <10fa5f6ba64b354b99b0f7b372e66e45bb4d9379.1713033988.git.mprivozn@redhat.com>
In-Reply-To: <2127032ed8cd49001465dc0dce9f842e13467bc2.1713033988.git.mprivozn@redhat.com>
References: <2127032ed8cd49001465dc0dce9f842e13467bc2.1713033988.git.mprivozn@redhat.com>
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Fri, 15 Mar 2024 10:47:50 +0000
Subject: [PATCH 2/2] remote: check for negative array lengths before
allocation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

While the C API entry points will validate non-negative lengths
for various parameters, the RPC server de-serialization code
will need to allocate memory for arrays before entering the C
API. These allocations will thus happen before the non-negative
length check is performed.

Passing a negative length to the g_new0 function will usually
result in a crash due to the negative length being treated as
a huge positive number.

This was found and diagnosed by ALT Linux Team with AFLplusplus.

CVE-2024-2494
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Found-by: Alexandr Shashkin <dutyrok@altlinux.org>
Co-developed-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 8a3f8d957507c1f8223fdcf25a3ff885b15557f2)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
src/remote/remote_daemon_dispatch.c | 65 +++++++++++++++++++++++++++++
src/rpc/gendispatch.pl | 5 +++
2 files changed, 70 insertions(+)

diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 7daf503b51..7542caa952 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2291,6 +2291,10 @@ remoteDispatchDomainGetSchedulerParameters(virNetServer *server G_GNUC_UNUSED,
if (!conn)
goto cleanup;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -2339,6 +2343,10 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServer *server G_GNUC_UNUS
if (!conn)
goto cleanup;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -2497,6 +2505,10 @@ remoteDispatchDomainBlockStatsFlags(virNetServer *server G_GNUC_UNUSED,
goto cleanup;
flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -2717,6 +2729,14 @@ remoteDispatchDomainGetVcpuPinInfo(virNetServer *server G_GNUC_UNUSED,
if (!(dom = get_nonnull_domain(conn, args->dom)))
goto cleanup;

+ if (args->ncpumaps < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpumaps must be non-negative"));
+ goto cleanup;
+ }
+ if (args->maplen < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maplen must be non-negative"));
+ goto cleanup;
+ }
if (args->ncpumaps > REMOTE_VCPUINFO_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpumaps > REMOTE_VCPUINFO_MAX"));
goto cleanup;
@@ -2811,6 +2831,11 @@ remoteDispatchDomainGetEmulatorPinInfo(virNetServer *server G_GNUC_UNUSED,
if (!(dom = get_nonnull_domain(conn, args->dom)))
goto cleanup;

+ if (args->maplen < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maplen must be non-negative"));
+ goto cleanup;
+ }
+
/* Allocate buffers to take the results */
if (args->maplen > 0)
cpumaps = g_new0(unsigned char, args->maplen);
@@ -2858,6 +2883,14 @@ remoteDispatchDomainGetVcpus(virNetServer *server G_GNUC_UNUSED,
if (!(dom = get_nonnull_domain(conn, args->dom)))
goto cleanup;

+ if (args->maxinfo < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo must be non-negative"));
+ goto cleanup;
+ }
+ if (args->maplen < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo must be non-negative"));
+ goto cleanup;
+ }
if (args->maxinfo > REMOTE_VCPUINFO_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo > REMOTE_VCPUINFO_MAX"));
goto cleanup;
@@ -3096,6 +3129,10 @@ remoteDispatchDomainGetMemoryParameters(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -3156,6 +3193,10 @@ remoteDispatchDomainGetNumaParameters(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_NUMA_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -3216,6 +3257,10 @@ remoteDispatchDomainGetBlkioParameters(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -3277,6 +3322,10 @@ remoteDispatchNodeGetCPUStats(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_NODE_CPU_STATS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -3339,6 +3388,10 @@ remoteDispatchNodeGetMemoryStats(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_NODE_MEMORY_STATS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -3514,6 +3567,10 @@ remoteDispatchDomainGetBlockIoTune(virNetServer *server G_GNUC_UNUSED,
if (!conn)
goto cleanup;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -5079,6 +5136,10 @@ remoteDispatchDomainGetInterfaceParameters(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
@@ -5299,6 +5360,10 @@ remoteDispatchNodeGetMemoryParameters(virNetServer *server G_GNUC_UNUSED,

flags = args->flags;

+ if (args->nparams < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative"));
+ goto cleanup;
+ }
if (args->nparams > REMOTE_NODE_MEMORY_PARAMETERS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large"));
goto cleanup;
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index fa45d15a92..294e21f8a1 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1070,6 +1070,11 @@ elsif ($mode eq "server") {
print "\n";

if ($single_ret_as_list) {
+ print " if (args->$single_ret_list_max_var < 0) {\n";
+ print " virReportError(VIR_ERR_RPC,\n";
+ print " \"%s\", _(\"max$single_ret_list_name must be non-negative\"));\n";
+ print " goto cleanup;\n";
+ print " }\n";
print " if (args->$single_ret_list_max_var > $single_ret_list_max_define) {\n";
print " virReportError(VIR_ERR_RPC,\n";
print " \"%s\", _(\"max$single_ret_list_name > $single_ret_list_max_define\"));\n";
--
2.43.2

Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ PATCHES=(
"${FILESDIR}"/${PN}-9.4.0-fix_paths_in_libvirt-guests_sh.patch
"${FILESDIR}"/${PN}-9.9.0-do-not-use-sysconfig.patch
"${FILESDIR}"/${PN}-9.6.0-fix-paths-for-apparmor.patch
"${FILESDIR}"/${PN}-10.1.0-Fix-off-by-one-error-in-udevListInterfacesByStatus.patch
"${FILESDIR}"/${PN}-10.2.0-remote-check-for-negative-array-lengths-before-alloc.patch
)

pkg_setup() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ PATCHES=(
"${FILESDIR}"/${PN}-9.4.0-fix_paths_in_libvirt-guests_sh.patch
"${FILESDIR}"/${PN}-9.9.0-do-not-use-sysconfig.patch
"${FILESDIR}"/${PN}-9.6.0-fix-paths-for-apparmor.patch
"${FILESDIR}"/${PN}-10.2.0-remote-check-for-negative-array-lengths-before-alloc.patch
)

pkg_setup() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ PATCHES=(
"${FILESDIR}"/${PN}-9.4.0-do-not-use-sysconfig.patch
"${FILESDIR}"/${PN}-9.6.0-fix-paths-for-apparmor.patch
"${FILESDIR}"/${PN}-9.10.0-virxml-include-libxml-xmlsave.h-for-xmlIndentTreeOut.patch
"${FILESDIR}"/${PN}-10.1.0-Fix-off-by-one-error-in-udevListInterfacesByStatus.patch
"${FILESDIR}"/${PN}-10.2.0-remote-check-for-negative-array-lengths-before-alloc.patch
)

pkg_setup() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ PATCHES=(
"${FILESDIR}"/${PN}-9.9.0-do-not-use-sysconfig.patch
"${FILESDIR}"/${PN}-9.6.0-fix-paths-for-apparmor.patch
"${FILESDIR}"/${PN}-9.10.0-virxml-include-libxml-xmlsave.h-for-xmlIndentTreeOut.patch
"${FILESDIR}"/${PN}-10.1.0-Fix-off-by-one-error-in-udevListInterfacesByStatus.patch
"${FILESDIR}"/${PN}-10.2.0-remote-check-for-negative-array-lengths-before-alloc.patch
)

pkg_setup() {
Expand Down