[PATCH] wbemprox: Return WBEM_E_ACCESS_DENIED from IEnumWbemClassObject::Next() if the object is empty.
I have an installer that goes into an infinite loop if IEnumWbemClassObject::Next() returns either WBEM_S_FALSE or WBEM_S_TIMEDOUT for an empty object when the installer queries for a missing KB update. Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/wbemprox/class.c | 1 + dlls/wbemprox/tests/query.c | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/dlls/wbemprox/class.c b/dlls/wbemprox/class.c index 042ebebdcd..e4ab2624fe 100644 --- a/dlls/wbemprox/class.c +++ b/dlls/wbemprox/class.c @@ -123,6 +123,7 @@ static HRESULT WINAPI enum_class_object_Next( if (lTimeout != WBEM_INFINITE && !once++) FIXME("timeout not supported\n"); *puReturned = 0; + if (!view->count) return WBEM_E_ACCESS_DENIED; if (ec->index >= view->count) return WBEM_S_FALSE; hr = create_class_object( view->table->name, iface, ec->index, NULL, apObjects ); diff --git a/dlls/wbemprox/tests/query.c b/dlls/wbemprox/tests/query.c index 2c1ca7f91d..a03c75f9d1 100644 --- a/dlls/wbemprox/tests/query.c +++ b/dlls/wbemprox/tests/query.c @@ -1566,6 +1566,31 @@ static void test_Win32_PnPEntity( IWbemServices *services ) IEnumWbemClassObject_Release( enm ); } +static void test_Win32_QuickFixEngineering( IWbemServices *services ) +{ + static const WCHAR queryW[] = + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ', + 'W','i','n','3','2','_','Q','u','i','c','k','F','i','x','E','n','g','i','n','e','e','r','i','n','g',' ', + 'W','H','E','R','E',' ','H','o','t','F','i','x','I','D','=','\"','0','x','d','e','a','d','b','e','e','f','\"',0}; + BSTR wql = SysAllocString( wqlW ), query = SysAllocString( queryW ); + HRESULT hr; + IEnumWbemClassObject *result; + IWbemClassObject *obj; + ULONG count; + + hr = IWbemServices_ExecQuery( services, wql, query, 0, NULL, &result ); + ok( hr == S_OK, "got %08x\n", hr ); + + count = 0xdeadbeef; + hr = IEnumWbemClassObject_Next( result, 1000, 1, &obj, &count ); + ok( hr == WBEM_E_ACCESS_DENIED || broken(hr == WBEM_S_FALSE || hr == WBEM_S_TIMEDOUT) /* before Win7 SP1 */, "got %08x\n", hr ); + ok( !count, "got %u\n", count ); + + IEnumWbemClassObject_Release( result ); + SysFreeString( wql ); + SysFreeString( query ); +} + START_TEST(query) { static const WCHAR cimv2W[] = {'R','O','O','T','\\','C','I','M','V','2',0}; @@ -1613,6 +1638,7 @@ START_TEST(query) test_Win32_VideoController( services ); test_Win32_Printer( services ); test_Win32_PnPEntity( services ); + test_Win32_QuickFixEngineering( services ); SysFreeString( path ); IWbemServices_Release( services ); -- 2.20.1
On Mon, 2019-07-08 at 13:07 +0800, Dmitry Timoshkov wrote:
+static void test_Win32_QuickFixEngineering( IWbemServices *services ) +{ + static const WCHAR queryW[] = + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ', + 'W','i','n','3','2','_','Q','u','i','c','k','F','i','x','E','n','g','i','n','e','e','r','i','n','g',' ', + 'W','H','E','R','E',' ','H','o','t','F','i','x','I','D','=','\"','0','x','d','e','a','d','b','e','e','f','\"',0}; + BSTR wql = SysAllocString( wqlW ), query = SysAllocString( queryW ); + HRESULT hr; + IEnumWbemClassObject *result; + IWbemClassObject *obj; + ULONG count; + + hr = IWbemServices_ExecQuery( services, wql, query, 0, NULL, &result ); + ok( hr == S_OK, "got %08x\n", hr ); + + count = 0xdeadbeef; + hr = IEnumWbemClassObject_Next( result, 1000, 1, &obj, &count ); + ok( hr == WBEM_E_ACCESS_DENIED || broken(hr == WBEM_S_FALSE || hr == WBEM_S_TIMEDOUT) /* before Win7 SP1 */, "got %08x\n", hr ); + ok( !count, "got %u\n", count );
I don't see this. Even up-to-date Windows 10 returns WBEM_S_FALSE here.
Hans Leidekker <hans(a)codeweavers.com> wrote:
On Mon, 2019-07-08 at 13:07 +0800, Dmitry Timoshkov wrote:
+static void test_Win32_QuickFixEngineering( IWbemServices *services ) +{ + static const WCHAR queryW[] = + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ', + 'W','i','n','3','2','_','Q','u','i','c','k','F','i','x','E','n','g','i','n','e','e','r','i','n','g',' ', + 'W','H','E','R','E',' ','H','o','t','F','i','x','I','D','=','\"','0','x','d','e','a','d','b','e','e','f','\"',0}; + BSTR wql = SysAllocString( wqlW ), query = SysAllocString( queryW ); + HRESULT hr; + IEnumWbemClassObject *result; + IWbemClassObject *obj; + ULONG count; + + hr = IWbemServices_ExecQuery( services, wql, query, 0, NULL, &result ); + ok( hr == S_OK, "got %08x\n", hr ); + + count = 0xdeadbeef; + hr = IEnumWbemClassObject_Next( result, 1000, 1, &obj, &count ); + ok( hr == WBEM_E_ACCESS_DENIED || broken(hr == WBEM_S_FALSE || hr == WBEM_S_TIMEDOUT) /* before Win7 SP1 */, "got %08x\n", hr ); + ok( !count, "got %u\n", count );
I don't see this. Even up-to-date Windows 10 returns WBEM_S_FALSE here.
Looks like this is a result of executing from an MSI custom action. Is there a way to detect such a case and return an appropriate error value? -- Dmitry.
On Mon, 2019-07-08 at 17:04 +0800, Dmitry Timoshkov wrote:
Hans Leidekker <hans(a)codeweavers.com> wrote:
On Mon, 2019-07-08 at 13:07 +0800, Dmitry Timoshkov wrote:
+static void test_Win32_QuickFixEngineering( IWbemServices *services ) +{ + static const WCHAR queryW[] = + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ', + 'W','i','n','3','2','_','Q','u','i','c','k','F','i','x','E','n','g','i','n','e','e','r','i','n','g',' ', + 'W','H','E','R','E',' ','H','o','t','F','i','x','I','D','=','\"','0','x','d','e','a','d','b','e','e','f','\"',0}; + BSTR wql = SysAllocString( wqlW ), query = SysAllocString( queryW ); + HRESULT hr; + IEnumWbemClassObject *result; + IWbemClassObject *obj; + ULONG count; + + hr = IWbemServices_ExecQuery( services, wql, query, 0, NULL, &result ); + ok( hr == S_OK, "got %08x\n", hr ); + + count = 0xdeadbeef; + hr = IEnumWbemClassObject_Next( result, 1000, 1, &obj, &count ); + ok( hr == WBEM_E_ACCESS_DENIED || broken(hr == WBEM_S_FALSE || hr == WBEM_S_TIMEDOUT) /* before Win7 SP1 */, "got %08x\n", hr ); + ok( !count, "got %u\n", count );
I don't see this. Even up-to-date Windows 10 returns WBEM_S_FALSE here.
Looks like this is a result of executing from an MSI custom action. Is there a way to detect such a case and return an appropriate error value?
The error code suggests that the custom action is run with insufficient permissions. If that's the case we'd need to do the same and add a permission check somewhere.
Hans Leidekker <hans(a)codeweavers.com> wrote:
+static void test_Win32_QuickFixEngineering( IWbemServices *services ) +{ + static const WCHAR queryW[] = + {'S','E','L','E','C','T',' ','*',' ','F','R','O','M',' ', + 'W','i','n','3','2','_','Q','u','i','c','k','F','i','x','E','n','g','i','n','e','e','r','i','n','g',' ', + 'W','H','E','R','E',' ','H','o','t','F','i','x','I','D','=','\"','0','x','d','e','a','d','b','e','e','f','\"',0}; + BSTR wql = SysAllocString( wqlW ), query = SysAllocString( queryW ); + HRESULT hr; + IEnumWbemClassObject *result; + IWbemClassObject *obj; + ULONG count; + + hr = IWbemServices_ExecQuery( services, wql, query, 0, NULL, &result ); + ok( hr == S_OK, "got %08x\n", hr ); + + count = 0xdeadbeef; + hr = IEnumWbemClassObject_Next( result, 1000, 1, &obj, &count ); + ok( hr == WBEM_E_ACCESS_DENIED || broken(hr == WBEM_S_FALSE || hr == WBEM_S_TIMEDOUT) /* before Win7 SP1 */, "got %08x\n", hr ); + ok( !count, "got %u\n", count );
I don't see this. Even up-to-date Windows 10 returns WBEM_S_FALSE here.
Looks like this is a result of executing from an MSI custom action. Is there a way to detect such a case and return an appropriate error value?
The error code suggests that the custom action is run with insufficient permissions. If that's the case we'd need to do the same and add a permission check somewhere.
Under Windows MSI custom action is executed in the context of an MSI server, which probably runs as a restricted process, and which doesn't exist in Wine. Could you suggest a place in Wine where a permission check could be added? -- Dmitry.
On Mon, 2019-07-08 at 17:37 +0800, Dmitry Timoshkov wrote:
Looks like this is a result of executing from an MSI custom action.
Is there a way to detect such a case and return an appropriate error value?
The error code suggests that the custom action is run with insufficient permissions. If that's the case we'd need to do the same and add a permission check somewhere.
Under Windows MSI custom action is executed in the context of an MSI server, which probably runs as a restricted process, and which doesn't exist in Wine.
Custom actions are executed in a seperate process in Wine.
Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
Hans Leidekker <hans(a)codeweavers.com> wrote:
On Mon, 2019-07-08 at 17:37 +0800, Dmitry Timoshkov wrote:
Looks like this is a result of executing from an MSI custom action.
Is there a way to detect such a case and return an appropriate error value?
The error code suggests that the custom action is run with insufficient permissions. If that's the case we'd need to do the same and add a permission check somewhere.
Under Windows MSI custom action is executed in the context of an MSI server, which probably runs as a restricted process, and which doesn't exist in Wine.
Custom actions are executed in a seperate process in Wine.
Yes, but that's obviously not an MSI server which probably executes under a resticted user account in Windows.
Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
I'll copy WMI tests from dlls/wbemprox/tests to the MSI custom action tests to get a picture how it works under Windows. -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
I'll copy WMI tests from dlls/wbemprox/tests to the MSI custom action tests to get a picture how it works under Windows.
I've ported dlls/wbemprox/query.c to run from the MSI custom action, added strict checks for ::Next() return values everywhere, and while some of the queries fail ::Next() never returns WBEM_E_ACCESS_DENIED. It turned out that commenting out CoSetProxyBlanket() made the ::Next() calls for empty objects always return WBEM_E_ACCESS_DENIED instead of WBEM_S_FALSE/WBEM_S_TIMEDOUT. It's not obvious to me where the proposed access check should be added. Hans, do the test results add any clue to you? -- Dmitry.
On Mon, 2019-07-08 at 20:09 +0800, Dmitry Timoshkov wrote:
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
I'll copy WMI tests from dlls/wbemprox/tests to the MSI custom action tests to get a picture how it works under Windows.
I've ported dlls/wbemprox/query.c to run from the MSI custom action, added strict checks for ::Next() return values everywhere, and while some of the queries fail ::Next() never returns WBEM_E_ACCESS_DENIED. It turned out that commenting out CoSetProxyBlanket() made the ::Next() calls for empty objects always return WBEM_E_ACCESS_DENIED instead of WBEM_S_FALSE/WBEM_S_TIMEDOUT. It's not obvious to me where the proposed access check should be added. Hans, do the test results add any clue to you?
Commenting out the CoSetProxyBlanket call in the wbemprox tests doesn't change anything however. We still don't know what's different when running as a custom action.
Hans Leidekker <hans(a)codeweavers.com> wrote:
On Mon, 2019-07-08 at 20:09 +0800, Dmitry Timoshkov wrote:
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
I'll copy WMI tests from dlls/wbemprox/tests to the MSI custom action tests to get a picture how it works under Windows.
I've ported dlls/wbemprox/query.c to run from the MSI custom action, added strict checks for ::Next() return values everywhere, and while some of the queries fail ::Next() never returns WBEM_E_ACCESS_DENIED. It turned out that commenting out CoSetProxyBlanket() made the ::Next() calls for empty objects always return WBEM_E_ACCESS_DENIED instead of WBEM_S_FALSE/WBEM_S_TIMEDOUT. It's not obvious to me where the proposed access check should be added. Hans, do the test results add any clue to you?
Commenting out the CoSetProxyBlanket call in the wbemprox tests doesn't change anything however.
Yes, sorry for not mentioning that, since I'm testing on both sides.
We still don't know what's different when running as a custom action.
What exactly you would like to know? Also if you have suggestions for further tests and things to examine that would help a lot. -- Dmitry.
On Mon, 2019-07-08 at 20:56 +0800, Dmitry Timoshkov wrote:
Hans Leidekker <hans(a)codeweavers.com> wrote:
On Mon, 2019-07-08 at 20:09 +0800, Dmitry Timoshkov wrote:
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
I'll copy WMI tests from dlls/wbemprox/tests to the MSI custom action tests to get a picture how it works under Windows.
I've ported dlls/wbemprox/query.c to run from the MSI custom action, added strict checks for ::Next() return values everywhere, and while some of the queries fail ::Next() never returns WBEM_E_ACCESS_DENIED. It turned out that commenting out CoSetProxyBlanket() made the ::Next() calls for empty objects always return WBEM_E_ACCESS_DENIED instead of WBEM_S_FALSE/WBEM_S_TIMEDOUT. It's not obvious to me where the proposed access check should be added. Hans, do the test results add any clue to you?
Commenting out the CoSetProxyBlanket call in the wbemprox tests doesn't change anything however.
Yes, sorry for not mentioning that, since I'm testing on both sides.
We still don't know what's different when running as a custom action.
What exactly you would like to know? Also if you have suggestions for further tests and things to examine that would help a lot.
Check the process token, for example, and compare properties/privileges.
Hans Leidekker <hans(a)codeweavers.com> wrote:
> Could you suggest a place in Wine where a permission check could be added?
It's not clear to me if this is a general WMI thing or if it depends on the object being queried.
I'll copy WMI tests from dlls/wbemprox/tests to the MSI custom action tests to get a picture how it works under Windows.
I've ported dlls/wbemprox/query.c to run from the MSI custom action, added strict checks for ::Next() return values everywhere, and while some of the queries fail ::Next() never returns WBEM_E_ACCESS_DENIED. It turned out that commenting out CoSetProxyBlanket() made the ::Next() calls for empty objects always return WBEM_E_ACCESS_DENIED instead of WBEM_S_FALSE/WBEM_S_TIMEDOUT. It's not obvious to me where the proposed access check should be added. Hans, do the test results add any clue to you?
Commenting out the CoSetProxyBlanket call in the wbemprox tests doesn't change anything however.
Yes, sorry for not mentioning that, since I'm testing on both sides.
We still don't know what's different when running as a custom action.
What exactly you would like to know? Also if you have suggestions for further tests and things to examine that would help a lot.
Check the process token, for example, and compare properties/privileges.
According to my tests and investigation using Process Explorer the call to CoSetProxyBlanket() doesn't change anything in the process token. On the other hand MSDN description https://docs.microsoft.com/en-us/windows/win32/wmisdk/setting-the-security-l... implies that CoSetProxyBlanket() changes the COM security for the specified proxy, so it has nothing to do with process security. It's not clear what else could be done, any suggestions Hans? -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Hans Leidekker