From 88288c2a51faa7c795f053fc8b31b1c16ff804c5 Mon Sep 17 00:00:00 2001 From: Konstantin Kostiuk Date: Fri, 3 Mar 2023 21:20:07 +0200 Subject: qga/win32: Remove change action from MSI installer Remove the 'change' button from "Programs and Features" because it does not checks if a user is an admin or not. The installer has no components to choose from and always installs everything. So the 'change' button is not obviously needed but can create a security issue. resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167423 fixes: CVE-2023-0664 (part 1 of 2) Signed-off-by: Konstantin Kostiuk Reviewed-by: Yan Vugenfirer Reported-by: Brian Wiltse --- qga/installer/qemu-ga.wxs | 1 + 1 file changed, 1 insertion(+) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index 51340f7ecc..feb629ec47 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -31,6 +31,7 @@ /> + -- cgit v1.2.3 From 07ce178a2b0768eb9e712bb5ad0cf6dc7fcf0158 Mon Sep 17 00:00:00 2001 From: Konstantin Kostiuk Date: Fri, 3 Mar 2023 21:20:08 +0200 Subject: qga/win32: Use rundll for VSS installation The custom action uses cmd.exe to run VSS Service installation and removal which causes an interactive command shell to spawn. This shell can be used to execute any commands as a SYSTEM user. Even if call qemu-ga.exe directly the interactive command shell will be spawned as qemu-ga.exe is a console application and used by users from the console as well as a service. As VSS Service runs from DLL which contains the installer and uninstaller code, it can be run directly by rundll32.exe without any interactive command shell. Add specific entry points for rundll which is just a wrapper for COMRegister/COMUnregister functions with proper arguments. resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167423 fixes: CVE-2023-0664 (part 2 of 2) Signed-off-by: Konstantin Kostiuk Reviewed-by: Yan Vugenfirer Reported-by: Brian Wiltse --- qga/installer/qemu-ga.wxs | 10 +++++----- qga/vss-win32/install.cpp | 9 +++++++++ qga/vss-win32/qga-vss.def | 2 ++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index feb629ec47..46ae9e7a13 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -127,22 +127,22 @@ - + diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp index b57508fbe0..68662a6dfc 100644 --- a/qga/vss-win32/install.cpp +++ b/qga/vss-win32/install.cpp @@ -357,6 +357,15 @@ out: return hr; } +STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int) +{ + COMRegister(); +} + +STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int) +{ + COMUnregister(); +} static BOOL CreateRegistryKey(LPCTSTR key, LPCTSTR value, LPCTSTR data) { diff --git a/qga/vss-win32/qga-vss.def b/qga/vss-win32/qga-vss.def index 927782c31b..ee97a81427 100644 --- a/qga/vss-win32/qga-vss.def +++ b/qga/vss-win32/qga-vss.def @@ -1,6 +1,8 @@ LIBRARY "QGA-PROVIDER.DLL" EXPORTS + DLLCOMRegister + DLLCOMUnregister COMRegister PRIVATE COMUnregister PRIVATE DllCanUnloadNow PRIVATE -- cgit v1.2.3 From fe67fc0eb47834840c54a065c99b55b5ff5120a6 Mon Sep 17 00:00:00 2001 From: Kfir Manor Date: Thu, 9 Feb 2023 10:50:02 +0200 Subject: qga/win/installer: add VssOption to installer Adds registry value VssOption with value 1 to QEMU Guest Agent VSS Provider service registry key Signed-off-by: Kfir Manor Reviewed-by: Konstantin Kostiuk Signed-off-by: Konstantin Kostiuk --- qga/installer/qemu-ga.wxs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index 46ae9e7a13..df572adb4a 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -122,6 +122,10 @@ + + + -- cgit v1.2.3 From 410542d4a2d7c1d8136d3e49fc3ca29fbb76789a Mon Sep 17 00:00:00 2001 From: Kfir Manor Date: Thu, 9 Feb 2023 10:50:03 +0200 Subject: qga/win/vss: query VSS backup type Query VSS backup type number (DWORD) from QEMU Guest Agent VSS Provider registry key registry value VssOption Translate the VSS backup type number (DWORD) into its VSS backup type (VSS_BACKUP_TYPE) Returns the queried VSS backup type if the program encounters unexpected behaviors or values return default VSS backup type VSS_BT_FULL instead Signed-off-by: Kfir Manor Reviewed-by: Konstantin Kostiuk Signed-off-by: Konstantin Kostiuk --- qga/vss-win32/requester.cpp | 38 ++++++++++++++++++++++++++++++++++++++ qga/vss-win32/vss-handles.h | 3 +++ 2 files changed, 41 insertions(+) diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp index b371affeab..e06d516675 100644 --- a/qga/vss-win32/requester.cpp +++ b/qga/vss-win32/requester.cpp @@ -23,6 +23,8 @@ /* Call QueryStatus every 10 ms while waiting for frozen event */ #define VSS_TIMEOUT_EVENT_MSEC 10 +#define DEFAULT_VSS_BACKUP_TYPE VSS_BT_FULL + #define err_set(e, err, fmt, ...) \ ((e)->error_setg_win32_wrapper((e)->errp, __FILE__, __LINE__, __func__, \ err, fmt, ## __VA_ARGS__)) @@ -234,6 +236,42 @@ out: } } +DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR valueName, + DWORD defaultData) +{ + DWORD regGetValueError; + DWORD dwordData; + DWORD dataSize = sizeof(DWORD); + + regGetValueError = RegGetValue(baseKey, subKey, valueName, RRF_RT_DWORD, + NULL, &dwordData, &dataSize); + if (regGetValueError != ERROR_SUCCESS) { + return defaultData; + } + return dwordData; +} + +bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT) +{ + return (vssBT > VSS_BT_UNDEFINED && vssBT < VSS_BT_OTHER); +} + +VSS_BACKUP_TYPE get_vss_backup_type( + VSS_BACKUP_TYPE defaultVssBT = DEFAULT_VSS_BACKUP_TYPE) +{ + VSS_BACKUP_TYPE vssBackupType; + + vssBackupType = static_cast( + get_reg_dword_value(HKEY_LOCAL_MACHINE, + QGA_PROVIDER_REGISTRY_ADDRESS, + "VssOption", + defaultVssBT)); + if (!is_valid_vss_backup_type(vssBackupType)) { + return defaultVssBT; + } + return vssBackupType; +} + void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset) { COMPointer pAsync; diff --git a/qga/vss-win32/vss-handles.h b/qga/vss-win32/vss-handles.h index 0f8a741ad2..1a7d842129 100644 --- a/qga/vss-win32/vss-handles.h +++ b/qga/vss-win32/vss-handles.h @@ -6,6 +6,9 @@ #define QGA_PROVIDER_NAME "QEMU Guest Agent VSS Provider" #define QGA_PROVIDER_LNAME L(QGA_PROVIDER_NAME) #define QGA_PROVIDER_VERSION L(QEMU_VERSION) +#define QGA_PROVIDER_REGISTRY_ADDRESS "SYSTEM\\CurrentControlSet"\ + "\\Services"\ + "\\" QGA_PROVIDER_NAME #define EVENT_NAME_FROZEN "Global\\QGAVSSEvent-frozen" #define EVENT_NAME_THAW "Global\\QGAVSSEvent-thaw" -- cgit v1.2.3 From 0961f929c66ceb5e9e95756bfe418b9ef34510eb Mon Sep 17 00:00:00 2001 From: Kfir Manor Date: Thu, 9 Feb 2023 10:50:04 +0200 Subject: qga/win/vss: requester_freeze changes Change requester_freeze so that the VSS backup type queried from the registry Signed-off-by: Kfir Manor Reviewed-by: Konstantin Kostiuk Signed-off-by: Konstantin Kostiuk --- qga/vss-win32/requester.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp index e06d516675..3e998af4a8 100644 --- a/qga/vss-win32/requester.cpp +++ b/qga/vss-win32/requester.cpp @@ -285,6 +285,7 @@ void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset) DWORD wait_status; int num_fixed_drives = 0, i; int num_mount_points = 0; + VSS_BACKUP_TYPE vss_bt = get_vss_backup_type(); if (vss_ctx.pVssbc) { /* already frozen */ *num_vols = 0; @@ -332,7 +333,7 @@ void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset) goto out; } - hr = vss_ctx.pVssbc->SetBackupState(true, true, VSS_BT_FULL, false); + hr = vss_ctx.pVssbc->SetBackupState(true, true, vss_bt, false); if (FAILED(hr)) { err_set(errset, hr, "failed to set backup state"); goto out; -- cgit v1.2.3