From 88288c2a51faa7c795f053fc8b31b1c16ff804c5 Mon Sep 17 00:00:00 2001
From: Konstantin Kostiuk <kkostiuk@redhat.com>
Date: Fri, 3 Mar 2023 21:20:07 +0200
Subject: [PATCH 1/5] 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 <kkostiuk@redhat.com>
Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>
Reported-by: Brian Wiltse <brian.wiltse@live.com>
---
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 @@
/>
<Media Id="1" Cabinet="qemu_ga.$(var.QEMU_GA_VERSION).cab" EmbedCab="yes" />
<Property Id="WHSLogo">1</Property>
+ <Property Id="ARPNOMODIFY" Value="yes" Secure="yes" />
<MajorUpgrade
DowngradeErrorMessage="Error: A newer version of QEMU guest agent is already installed."
/>
From 07ce178a2b0768eb9e712bb5ad0cf6dc7fcf0158 Mon Sep 17 00:00:00 2001
From: Konstantin Kostiuk <kkostiuk@redhat.com>
Date: Fri, 3 Mar 2023 21:20:08 +0200
Subject: [PATCH 2/5] 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 <kkostiuk@redhat.com>
Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>
Reported-by: Brian Wiltse <brian.wiltse@live.com>
---
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 @@
</Directory>
</Directory>
- <Property Id="cmd" Value="cmd.exe"/>
+ <Property Id="rundll" Value="rundll32.exe"/>
<Property Id="REINSTALLMODE" Value="amus"/>
<?ifdef var.InstallVss?>
<CustomAction Id="RegisterCom"
- ExeCommand='/c "[qemu_ga_directory]qemu-ga.exe" -s vss-install'
+ ExeCommand='"[qemu_ga_directory]qga-vss.dll",DLLCOMRegister'
Execute="deferred"
- Property="cmd"
+ Property="rundll"
Impersonate="no"
Return="check"
>
</CustomAction>
<CustomAction Id="UnRegisterCom"
- ExeCommand='/c "[qemu_ga_directory]qemu-ga.exe" -s vss-uninstall'
+ ExeCommand='"[qemu_ga_directory]qga-vss.dll",DLLCOMUnregister'
Execute="deferred"
- Property="cmd"
+ Property="rundll"
Impersonate="no"
Return="check"
>
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
From fe67fc0eb47834840c54a065c99b55b5ff5120a6 Mon Sep 17 00:00:00 2001
From: Kfir Manor <kfir@daynix.com>
Date: Thu, 9 Feb 2023 10:50:02 +0200
Subject: [PATCH 3/5] 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 <kfir@daynix.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
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 @@
<RegistryValue Type="integer" Name="TypesSupported" Value="7" />
<RegistryValue Type="string" Name="EventMessageFile" Value="[qemu_ga_directory]qemu-ga.exe" />
</RegistryKey>
+ <RegistryKey Root="HKLM"
+ Key="System\CurrentControlSet\Services\QEMU Guest Agent VSS Provider">
+ <RegistryValue Type="integer" Name="VssOption" Value="1" />
+ </RegistryKey>
</Component>
</Directory>
</Directory>
From 410542d4a2d7c1d8136d3e49fc3ca29fbb76789a Mon Sep 17 00:00:00 2001
From: Kfir Manor <kfir@daynix.com>
Date: Thu, 9 Feb 2023 10:50:03 +0200
Subject: [PATCH 4/5] 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 <kfir@daynix.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
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<VSS_BACKUP_TYPE>(
+ 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<IVssAsync> 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"
From 0961f929c66ceb5e9e95756bfe418b9ef34510eb Mon Sep 17 00:00:00 2001
From: Kfir Manor <kfir@daynix.com>
Date: Thu, 9 Feb 2023 10:50:04 +0200
Subject: [PATCH 5/5] qga/win/vss: requester_freeze changes
Change requester_freeze so that the VSS backup type queried from the registry
Signed-off-by: Kfir Manor <kfir@daynix.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
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;