version: Fix wrong length reported when selecting a block. (resend)

Mark Jansen learn0more+wine at gmail.com
Tue Feb 17 16:08:06 CST 2015


Hello,

sorry for the late reply.
attached is a patch that demonstrates windows handles this fine, and
wine does not.

https://testbot.winehq.org/JobDetails.pl?Key=11597




On Mon, Feb 16, 2015 at 5:05 PM, Matteo Bruni <matteo.mystral at gmail.com> wrote:
> 2015-02-13 20:41 GMT+01:00 Mark Jansen <learn0more+wine at gmail.com>:
>> Hello,
>>
>
> Hi,
>
>> VersionInfo32_QueryValue seeks the requested element in a resource,
>> and the check after that (which i patched) asks if this succeeded, and
>> if it succeeded if it is text.
>> Now there are some rare case where it succeeds, and where it is text
>> but the length is actually 0.
>> The data at this point should not matter, but because the check did
>> not take into account the length returned, and blindly fed the data to
>> WideCharToMultiByte (which uses some pointer trickery that in this
>> very specific case is wrong to calculate the length), the result is
>> VerQueryValueA telling you that it just found you x bytes of data,
>> where it should have reported 0. (x depending on the data encountered
>> at the current read position, usually 0 so usually it accidentally
>> gives the correct value back.
>
> The question is whether 0-length strings (i.e. not even the NULL
> terminator) can happen or not. It's not clear to me with a quick look
> at the tests.
> So, while I'm not AJ, I think a test would make this more convincing.
>
>> The 'dum' i introduced is purely to make sure the function doesnt trip
>> if someone gives it a NULL pointer.
>> regards,
>>
>> On Fri, Feb 13, 2015 at 11:27 AM, Stefan Dösinger
>> <stefandoesinger at gmail.com> wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> Am 2015-02-12 um 20:15 schrieb Mark Jansen:
>>>>          LPWSTR lpSubBlockW;
>>>> +        UINT dum = 0;
>>>> +        if (!puLen)
>>>> +            puLen = &dum;
>>>>
>>>>          len  = MultiByteToWideChar(CP_ACP, 0, lpSubBlock, -1, NULL, 0);
>>>>          lpSubBlockW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
>>>> @@ -965,7 +968,7 @@ BOOL WINAPI VerQueryValueA( LPCVOID pBlock, LPCSTR lpSubBlock,
>>>>
>>>>          HeapFree(GetProcessHeap(), 0, lpSubBlockW);
>>>>
>>>> -        if (ret && isText)
>>>> +        if (ret && isText && *puLen)
>>> I am afraid I have no idea what this patch is doing. I am not even sure
>>> I understand the original function properly :-( . VerQueryValueA seems
>>> like an A to W wrapper for VersionInfo32_QueryValue, but with the quirky
>>> behavior that it appends the ascii string after the unicode string.
>>>
>>> Can you elaborate a bit on what exactly goes wrong? What are the input
>>> values, and what does VersionInfo32_QueryValue return?
>>>
>>> -----BEGIN PGP SIGNATURE-----
>>> Version: GnuPG v2
>>>
>>> iQIcBAEBAgAGBQJU3dGbAAoJEN0/YqbEcdMwpwEP+wabxLJtbw7O7enIEu21l8ls
>>> m6vxZr1PXP5RnTmOs4nk/QmWT4Oa68qY2EBdArZ79WwyVULLI7BjNJ1aLS67dw/Z
>>> l6Dk9fh8orxoD4HznH0o7PxYrWNrQ1nJ2jf39k+TO4DbSHMZB/eGXnGUZxVFwKVf
>>> H7yNxmoSISC0aWgQP0/QZF+AhtsJ5TLIWtNyJULpmTnZZGu9z94Ukftd9gDL+tZU
>>> ezFsqxdDhpy0nirb4ky/Yq77jXQtsCmz/pCNi7ntYuQqY5fEmtJQMAtpfOQhdG5h
>>> XP1Py7qFTu/RgqnBsO1Z3INRyKk7NS4BsPvULcLsk4WbDvoTGEqa1WwHwqHY4Dng
>>> IDHagE938Xkc8CPiHGzqJG6WDdPPkFiRzqM+2HH87ihJ2uAaVZcK/V46lGiJCx8q
>>> jDY+DlcDyZRmw28+fjwzOjPQHZx/jH2X3wLIpkliM5Ja03Ocb27Aaud9DEQ6H0vw
>>> 0VbD2Db6oOrPBv3oyDmZhI+NTPgvsUoaH6sNE+3cIEwBWpCKkOJMzoDZ6EiqWVqj
>>> jvJklcVcOn7nfUBH0YCfnG5szQokMnfcnZaOZxzkjWnPvwJbfC36eLQyESkbtbKf
>>> Y2Z+mvbWslkhdeR0jKAmQNwfDuFcfvZflinem3U9xJAqBKEmicLeqJ0dtwUIJyC7
>>> TnJMMCP14EDaX0DZGHw3
>>> =94Ql
>>> -----END PGP SIGNATURE-----
>>
>>
-------------- next part --------------
diff --git a/dlls/version/tests/info.c b/dlls/version/tests/info.c
index 107cfac..526853a 100644
--- a/dlls/version/tests/info.c
+++ b/dlls/version/tests/info.c
@@ -574,6 +574,71 @@ static void test_VerQueryValueA(void)
     HeapFree(GetProcessHeap(), 0, ver);
 }
 
+static void test_VerQueryValueA_InvalidLength(void)
+{
+    char szPreparedBuffer[] = {
+        0x04, 0x01, 0x34, 0x00, 0x00, 0x00, 0x56, 0x00, 0x53, 0x00, 0x5f, 0x00, 0x56, 0x00, 0x45, 0x00,
+        0x52, 0x00, 0x53, 0x00, 0x49, 0x00, 0x4f, 0x00, 0x4e, 0x00, 0x5f, 0x00, 0x49, 0x00, 0x4e, 0x00,
+        0x46, 0x00, 0x4f, 0x00, 0x00, 0x00, 0x00, 0x00, 0xbd, 0x04, 0xef, 0xfe, 0x00, 0x00, 0x01, 0x00,
+        0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x3f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x64, 0x00, 0x00, 0x00,
+        0x01, 0x00, 0x53, 0x00, 0x74, 0x00, 0x72, 0x00, 0x69, 0x00, 0x6e, 0x00, 0x67, 0x00, 0x46, 0x00,
+        0x69, 0x00, 0x6c, 0x00, 0x65, 0x00, 0x49, 0x00, 0x6e, 0x00, 0x66, 0x00, 0x6f, 0x00, 0x00, 0x00,
+        0x40, 0x00, 0x00, 0x00, 0x01, 0x00, 0x30, 0x00, 0x34, 0x00, 0x30, 0x00, 0x39, 0x00, 0x30, 0x00,
+        0x34, 0x00, 0x45, 0x00, 0x34, 0x00, 0x00, 0x00, 0x28, 0x00, 0x08, 0x00, 0x01, 0x00, 0x50, 0x00,
+        0x72, 0x00, 0x6f, 0x00, 0x64, 0x00, 0x75, 0x00, 0x63, 0x00, 0x74, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x50, 0x00, 0x72, 0x00, 0x6f, 0x00, 0x64, 0x00, 0x75, 0x00, 0x63, 0x00, 0x74, 0x00, 0x00, 0x00,
+        0x44, 0x00, 0x00, 0x00, 0x01, 0x00, 0x56, 0x00, 0x61, 0x00, 0x72, 0x00, 0x46, 0x00, 0x69, 0x00,
+        0x6c, 0x00, 0x65, 0x00, 0x49, 0x00, 0x6e, 0x00, 0x66, 0x00, 0x6f, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x24, 0x00, 0x04, 0x00, 0x00, 0x00, 0x54, 0x00, 0x72, 0x00, 0x61, 0x00, 0x6e, 0x00, 0x73, 0x00,
+        0x6c, 0x00, 0x61, 0x00, 0x74, 0x00, 0x69, 0x00, 0x6f, 0x00, 0x6e, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x09, 0x04, 0xe4, 0x04, 0x46, 0x45, 0x32, 0x58, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba,
+        0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 0x0d, 0xf0, 0xad, 0xba, 
+    };
+    char *p;
+    UINT len, ret;
+
+    p = (char *)0xdeadbeef;
+    len = 0xdeadbeef;
+    SetLastError(0xdeadbeef);
+    ret = VerQueryValueA(szPreparedBuffer, "StringFileInfo", (LPVOID*)&p, &len);
+    ok(ret, "VerQueryValue error %u\n", GetLastError());
+    ok(len == 0, "VerQueryValue returned %u, expected 0\n", len);
+    ok(p != (char *)0xdeadbeef, "not expected 0xdeadbeef\n");
+
+    p = (char *)0xdeadbeef;
+    len = 0xdeadbeef;
+    SetLastError(0xdeadbeef);
+    ret = VerQueryValueA(szPreparedBuffer, "\\StringFileInfo", (LPVOID*)&p, &len);
+    ok(ret, "VerQueryValue error %u\n", GetLastError());
+    ok(len == 0, "VerQueryValue returned %u, expected 0\n", len);
+    ok(p != (char *)0xdeadbeef, "not expected 0xdeadbeef\n");
+
+    p = (char *)0xdeadbeef;
+    len = 0xdeadbeef;
+    SetLastError(0xdeadbeef);
+    ret = VerQueryValueA(szPreparedBuffer, "\\\\StringFileInfo", (LPVOID*)&p, &len);
+    ok(ret, "VerQueryValue error %u\n", GetLastError());
+    ok(len == 0, "VerQueryValue returned %u, expected 0\n", len);
+    ok(p != (char *)0xdeadbeef, "not expected 0xdeadbeef\n");
+}
+
 static void test_extra_block(void)
 {
     WORD extra_block[] = {
@@ -630,5 +695,6 @@ START_TEST(info)
     test_info();
     test_32bit_win();
     test_VerQueryValueA();
+    test_VerQueryValueA_InvalidLength();
     test_extra_block();
 }


More information about the wine-devel mailing list