From 9c0a00614b7bf47a6fa8e889a5943d2386f903ba Mon Sep 17 00:00:00 2001 From: caozhongwang Date: Wed, 10 Apr 2024 11:42:43 +0800 Subject: [PATCH 1/3] fix CVE-2024-1441 vish:Fix off-by-one error in udevListInterfacesByStatus (CVE-2024-1441) --- libvirt.spec | 6 ++- ...-one-error-in-udevListInterfacesBySt.patch | 39 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch diff --git a/libvirt.spec b/libvirt.spec index 091aa54..cf0b9b8 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -101,7 +101,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 6.2.0 -Release: 17 +Release: 18 License: LGPLv2+ URL: https://libvirt.org/ @@ -150,6 +150,7 @@ Patch0037: security-fix-SELinux-label-generation-logic.patch Patch0038: nwfilter-fix-crash-when-counting-number-of-network-f.patch Patch0039: qemu-Add-missing-lock-in-qemuProcessHandleMonitorEOF.patch Patch0040: update-the-Chinese-translation-of-nwfilter.patch +Patch0041: virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} @@ -1884,6 +1885,9 @@ exit 0 %changelog +* Wed Apr 10 2024 caozhongwang +- vish:Fix off-by-one error in udevListInterfacesByStatus (CVE-2024-1441) + * Sat Dec 10 2022 yezengruan - update the Chinese translation of nwfilter diff --git a/virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch b/virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch new file mode 100644 index 0000000..de70e00 --- /dev/null +++ b/virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch @@ -0,0 +1,39 @@ +From 001ede185f96d359481495a4016fcd0cffb2e1b0 Mon Sep 17 00:00:00 2001 +From: Martin Kletzander +Date: Tue, 27 Feb 2024 16:20:12 +0100 +Subject: [PATCH 6/8] virsh: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 +Reported-by: Alexander Kuznetsov +Fixes: 5a33366f5c0b18c93d161bd144f9f079de4ac8ca +Fixes: d6064e2759a24e0802f363e3a810dc5a7d7ebb15 +Reviewed-by: Ján Tomko +--- + 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 e388f98536..dde88860d3 100644 +--- a/src/interface/interface_backend_udev.c ++++ b/src/interface/interface_backend_udev.c +@@ -221,7 +221,7 @@ udevListInterfacesByStatus(virConnectPtr conn, + virInterfaceDefPtr def; + + /* 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.27.0 + -- Gitee From c093a5e233e638d531e4ddb1275f02363ec40fd7 Mon Sep 17 00:00:00 2001 From: caozhongwang Date: Wed, 10 Apr 2024 11:40:12 +0800 Subject: [PATCH 2/3] fix CVE-2024-2494 remote: check for negative array lengths before allocation (CVE-2024-2494) --- libvirt.spec | 6 +- ...-negative-array-lengths-before-alloc.patch | 216 ++++++++++++++++++ 2 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 remote-check-for-negative-array-lengths-before-alloc.patch diff --git a/libvirt.spec b/libvirt.spec index cf0b9b8..2487bd8 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -101,7 +101,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 6.2.0 -Release: 18 +Release: 19 License: LGPLv2+ URL: https://libvirt.org/ @@ -151,6 +151,7 @@ Patch0038: nwfilter-fix-crash-when-counting-number-of-network-f.patch Patch0039: qemu-Add-missing-lock-in-qemuProcessHandleMonitorEOF.patch Patch0040: update-the-Chinese-translation-of-nwfilter.patch Patch0041: virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch +Patch0042: remote-check-for-negative-array-lengths-before-alloc.patch Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} @@ -1885,6 +1886,9 @@ exit 0 %changelog +* Wed Apr 10 2024 caozhongwang +- remote: check for negative array lengths before allocation (CVE-2024-2494) + * Wed Apr 10 2024 caozhongwang - vish:Fix off-by-one error in udevListInterfacesByStatus (CVE-2024-1441) diff --git a/remote-check-for-negative-array-lengths-before-alloc.patch b/remote-check-for-negative-array-lengths-before-alloc.patch new file mode 100644 index 0000000..b8b2a09 --- /dev/null +++ b/remote-check-for-negative-array-lengths-before-alloc.patch @@ -0,0 +1,216 @@ +From e8245ba93fe1d7d345d10ff9a189b6f5188682b1 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= +Date: Fri, 15 Mar 2024 10:47:50 +0000 +Subject: [PATCH 7/8] 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. + +Reviewed-by: Michal Privoznik +Found-by: Alexandr Shashkin +Co-developed-by: Alexander Kuznetsov +Signed-off-by: Daniel P. Berrangé +--- + 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 c5506c2e11..5a1a317452 100644 +--- a/src/remote/remote_daemon_dispatch.c ++++ b/src/remote/remote_daemon_dispatch.c +@@ -2307,6 +2307,10 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr 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; +@@ -2355,6 +2359,10 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server G_GNUC_UN + 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; +@@ -2516,6 +2524,10 @@ remoteDispatchDomainBlockStatsFlags(virNetServerPtr 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; +@@ -2750,6 +2762,14 @@ remoteDispatchDomainGetVcpuPinInfo(virNetServerPtr 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; +@@ -2845,6 +2865,11 @@ remoteDispatchDomainGetEmulatorPinInfo(virNetServerPtr 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 && + VIR_ALLOC_N(cpumaps, args->maplen) < 0) +@@ -2893,6 +2918,14 @@ remoteDispatchDomainGetVcpus(virNetServerPtr 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; +@@ -3140,6 +3173,10 @@ remoteDispatchDomainGetMemoryParameters(virNetServerPtr 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; +@@ -3200,6 +3237,10 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr 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; +@@ -3260,6 +3301,10 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr 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; +@@ -3321,6 +3366,10 @@ remoteDispatchNodeGetCPUStats(virNetServerPtr 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; +@@ -3389,6 +3438,10 @@ remoteDispatchNodeGetMemoryStats(virNetServerPtr 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; +@@ -3570,6 +3623,10 @@ remoteDispatchDomainGetBlockIoTune(virNetServerPtr 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; +@@ -5128,6 +5185,10 @@ remoteDispatchDomainGetInterfaceParameters(virNetServerPtr 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; +@@ -5350,6 +5411,10 @@ remoteDispatchNodeGetMemoryParameters(virNetServerPtr 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 590a46ef66..02612f1856 100755 +--- a/src/rpc/gendispatch.pl ++++ b/src/rpc/gendispatch.pl +@@ -1074,6 +1074,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.27.0 + -- Gitee From 2ddb9bde8cf36d0b337ed5622da6f4597b31884a Mon Sep 17 00:00:00 2001 From: caozhongwang Date: Wed, 10 Apr 2024 11:51:41 +0800 Subject: [PATCH 3/3] fix CVE-2024-2496 interface: fix udev_device_get_sysattr_value return value check (CVE-2024-2496) --- ...ev_device_get_sysattr_value-return-v.patch | 89 +++++++++++++++++++ libvirt.spec | 6 +- 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 interface-fix-udev_device_get_sysattr_value-return-v.patch diff --git a/interface-fix-udev_device_get_sysattr_value-return-v.patch b/interface-fix-udev_device_get_sysattr_value-return-v.patch new file mode 100644 index 0000000..6747c81 --- /dev/null +++ b/interface-fix-udev_device_get_sysattr_value-return-v.patch @@ -0,0 +1,89 @@ +From 0d5c04534f0e041e9923da1cb37a431ae0d463a8 Mon Sep 17 00:00:00 2001 +From: Dmitry Frolov +Date: Tue, 12 Sep 2023 15:56:47 +0300 +Subject: [PATCH 8/8] interface: fix udev_device_get_sysattr_value return value + check + +Reviewing the code I found that return value of function +udev_device_get_sysattr_value() is dereferenced without a check. +udev_device_get_sysattr_value() may return NULL by number of reasons. + +v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE() +v3: More checks added, to skip earlier. More verbose VIR_DEBUG. + +Signed-off-by: Dmitry Frolov +Reviewed-by: Martin Kletzander +--- + src/interface/interface_backend_udev.c | 26 +++++++++++++++++++------- + 1 file changed, 19 insertions(+), 7 deletions(-) + +diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c +index dde88860d3..00eeee6cc4 100644 +--- a/src/interface/interface_backend_udev.c ++++ b/src/interface/interface_backend_udev.c +@@ -23,6 +23,7 @@ + #include + #include + ++#include "virlog.h" + #include "virerror.h" + #include "virfile.h" + #include "datatypes.h" +@@ -41,6 +42,8 @@ + + #define VIR_FROM_THIS VIR_FROM_INTERFACE + ++VIR_LOG_INIT("interface.interface_backend_udev"); ++ + struct udev_iface_driver { + struct udev *udev; + /* pid file FD, ensures two copies of the driver can't use the same root */ +@@ -371,11 +374,20 @@ udevConnectListAllInterfaces(virConnectPtr conn, + const char *macaddr; + virInterfaceDefPtr def; + +- path = udev_list_entry_get_name(dev_entry); +- dev = udev_device_new_from_syspath(udev, path); +- name = udev_device_get_sysname(dev); ++ if (!(path = udev_list_entry_get_name(dev_entry))) { ++ VIR_DEBUG("Skipping interface, path == NULL"); ++ continue; ++ } ++ if (!(dev = udev_device_new_from_syspath(udev, path))) { ++ VIR_DEBUG("Skipping interface '%s', dev == NULL", path); ++ continue; ++ } ++ if (!(name = udev_device_get_sysname(dev))) { ++ VIR_DEBUG("Skipping interface '%s', name == NULL", path); ++ continue; ++ } + macaddr = udev_device_get_sysattr_value(dev, "address"); +- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); ++ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); + + def = udevGetMinimalDefForDevice(dev); + if (!virConnectListAllInterfacesCheckACL(conn, def)) { +@@ -1000,9 +1012,9 @@ udevGetIfaceDef(struct udev *udev, const char *name) + + /* MTU */ + mtu_str = udev_device_get_sysattr_value(dev, "mtu"); +- if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { ++ if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, +- _("Could not parse MTU value '%s'"), mtu_str); ++ _("Could not parse MTU value '%s'"), NULLSTR(mtu_str)); + goto error; + } + ifacedef->mtu = mtu; +@@ -1129,7 +1141,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) + goto cleanup; + + /* Check if it's active or not */ +- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); ++ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); + + udev_device_unref(dev); + +-- +2.27.0 + diff --git a/libvirt.spec b/libvirt.spec index 2487bd8..de5096f 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -101,7 +101,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 6.2.0 -Release: 19 +Release: 20 License: LGPLv2+ URL: https://libvirt.org/ @@ -152,6 +152,7 @@ Patch0039: qemu-Add-missing-lock-in-qemuProcessHandleMonitorEOF.patch Patch0040: update-the-Chinese-translation-of-nwfilter.patch Patch0041: virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch Patch0042: remote-check-for-negative-array-lengths-before-alloc.patch +Patch0043: interface-fix-udev_device_get_sysattr_value-return-v.patch Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} @@ -1886,6 +1887,9 @@ exit 0 %changelog +* Wed Apr 10 2024 caozhongwang +- interface: fix udev_device_get_sysattr_value return value check (CVE-2024-2496) + * Wed Apr 10 2024 caozhongwang - remote: check for negative array lengths before allocation (CVE-2024-2494) -- Gitee