[PATCH 0/2] MR1263: kernelbase: Fix VerQueryValueW with no data

[Mark Jansen (＠learn-more)]

xul.dll from Mypal 27.9.4 has a resource entry without any data.
Windows returns this as null-terminated string, instead of pointing past the data.

The first commit adds a test,
the second commit fixes the bugs (and removes the todo_wine).

Original ticket for reference: https://jira.reactos.org/browse/CORE-15781

[PATCH 1/2] version/tests: Add a test for an empty value in VerQueryValueW

[Mark Jansen]

From: Mark Jansen

---
 dlls/version/tests/info.c | 145 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/dlls/version/tests/info.c b/dlls/version/tests/info.c
index 61ff05b2559..dcde1bf096f 100644
--- a/dlls/version/tests/info.c
+++ b/dlls/version/tests/info.c
@@ -572,6 +572,150 @@ static void test_VerQueryValueA(void)
     HeapFree(GetProcessHeap(), 0, ver);
 }

+/* taken from fusionpriv.h */
+    #include <pshpack1.h>
+typedef struct
+{
+    WORD wLength;
+    WORD wValueLength;
+    WORD wType;
+    WCHAR szKey[17];
+    VS_FIXEDFILEINFO Value;
+} VS_VERSIONINFO;
+
+typedef struct
+{
+    WORD wLength;
+    WORD wValueLength;
+    WORD wType;
+    WCHAR szKey[15];
+} STRINGFILEINFO;
+
+typedef struct
+{
+    WORD wLength;
+    WORD wValueLength;
+    WORD wType;
+    WCHAR szKey[9];
+} STRINGTABLE;
+
+typedef struct
+{
+    WORD wLength;
+    WORD wValueLength;
+    WORD wType;
+} STRINGHDR;
+
+typedef struct rsrc_section_t
+{
+    VS_VERSIONINFO version_info;
+    STRINGFILEINFO string_file_info;
+    STRINGTABLE string_table;
+
+    STRINGHDR FileVersion_hdr;
+    WCHAR FileVersion_key[13];
+
+    STRINGHDR ProductVersion_hdr;
+    WCHAR ProductVersion_key[15];
+    WCHAR ProductVersion_val[8];
+} rsrc_section_t;
+
+#include <poppack.h>
+
+#define    RT_VERSION_DW       16
+static const rsrc_section_t rsrc_section =
+{
+    /* version_info */
+    {
+    320, /* wLength */
+    0x34, /* wValueLength */
+    0, /* wType: Binary */
+    { 'V','S','_','V','E','R','S','I','O','N','_','I','N','F','O','\0','\0' }, /* szKey[17] */
+        /* Value */
+        {
+            0xFEEF04BD, /* dwSignature */
+            0x10000, /* dwStrucVersion */
+            0x10000, /* dwFileVersionMS */
+            0, /* dwFileVersionLS */
+            0x10000, /* dwProductVersionMS */
+            1, /* dwProductVersionLS */
+            0, /* dwFileFlagsMask */
+            0, /* dwFileFlags */
+            VOS__WINDOWS32, /* dwFileOS */
+            VFT_APP, /* dwFileType */
+            0, /* dwFileSubtype */
+            0x01d1a019, /* dwFileDateMS */
+            0xac754c50 /* dwFileDateLS */
+        },
+    },
+
+    /* string_file_info */
+    {
+        0x9E, /* wLength */
+        0, /* wValueLength */
+        1, /* wType: Text */
+        { 'S','t','r','i','n','g','F','i','l','e','I','n','f','o','\0' } /* szKey[15] */
+    },
+    /* string_table */
+    {
+        0x7A, /* wLength */
+        0, /* wValueLength */
+        1, /* wType: Text */
+        { 'F','F','F','F','0','0','0','0','\0' } /* szKey[9] */
+    },
+
+    /* FileVersion */
+    {
+        32, /* wLength */
+        0, /* wValueLength */
+        1, /* wType: Text */
+    },
+    { 'F','i','l','e','V','e','r','s','i','o','n','\0' },
+    /* There is no data here! */
+
+    /* ProductVersion */
+    {
+        52, /* wLength */
+        8, /* wValueLength */
+        1, /* wType: Text */
+    },
+    { 'P','r','o','d','u','c','t','V','e','r','s','i','o','n','\0' },
+    { '1','.','0','.','0','.','1','\0' },
+};
+
+static void test_VerQueryValue_EmptyData(void)
+{
+    char* p;
+    UINT len;
+    BOOL ret;
+    char* ver;
+
+    ver = HeapAlloc(GetProcessHeap(), 0, sizeof(rsrc_section) * 2);
+    ok(ver != NULL, "Can't allocate memory\n");
+    memcpy(ver, &rsrc_section, sizeof(rsrc_section));
+
+    /* Key without data */
+    p = (char *)0xdeadbeef;
+    len = 0xdeadbeef;
+    SetLastError(0xdeadbeef);
+    ret = VerQueryValueW(ver, L"\StringFileInfo\FFFF0000\FileVersion", (LPVOID *)&p, &len);
+    ok(ret, "VerQueryValueW error %u\n", GetLastError());
+    ok(len == 0, "VerQueryValueW returned %u, expected 0\n", len);
+    todo_wine
+    ok(p == (ver + offsetof(rsrc_section_t, FileVersion_key) + 11 * sizeof(WCHAR)), "p was %p, expected %p\n", p, ver + offsetof(rsrc_section_t, FileVersion_key) + 11 * sizeof(WCHAR));
+
+    /* The key behind it, to show that parsing continues just fine */
+    p = (char *)0xdeadbeef;
+    len = 0xdeadbeef;
+    SetLastError(0xdeadbeef);
+    ret = VerQueryValueW(ver, L"\StringFileInfo\FFFF0000\ProductVersion", (LPVOID *)&p, &len);
+    ok(ret, "VerQueryValueW error %u\n", GetLastError());
+    ok(len == 8, "VerQueryValueW returned %u, expected 0\n", len);
+    ok(p == (ver + offsetof(rsrc_section_t, ProductVersion_val)), "p was %p, expected %p\n", p, ver + offsetof(rsrc_section_t, ProductVersion_val));
+
+    HeapFree(GetProcessHeap(), 0, ver);
+}
+
 static void test_extra_block(void)
 {
     WORD extra_block[] = {
@@ -731,6 +875,7 @@ START_TEST(info)
     test_info();
     test_32bit_win();
     test_VerQueryValueA();
+    test_VerQueryValue_EmptyData();
     test_extra_block();
     test_GetFileVersionInfoEx();
 }

[PATCH 2/2] kernelbase: Fix VerQueryValueW with no data

[Mark Jansen]

From: Mark Jansen

---
 dlls/kernelbase/version.c | 7 ++++++-
 dlls/version/tests/info.c | 1 -
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/dlls/kernelbase/version.c b/dlls/kernelbase/version.c
index 4d5a8a4de93..e3f9ac917a9 100644
--- a/dlls/kernelbase/version.c
+++ b/dlls/kernelbase/version.c
@@ -1072,6 +1072,7 @@ static BOOL VersionInfo32_QueryValue( const VS_VERSION_INFO_STRUCT32 *info, LPCW
                                       LPVOID *lplpBuffer, UINT *puLen, BOOL *pbText )
 {
     TRACE("lpSubBlock : (%s)\n", debugstr_w(lpSubBlock));
+    PVOID Ptr;

while ( *lpSubBlock )
     {
@@ -1102,7 +1103,11 @@ static BOOL VersionInfo32_QueryValue( const VS_VERSION_INFO_STRUCT32 *info, LPCW
     }

/* Return value */
-    *lplpBuffer = VersionInfo32_Value( info );
+    Ptr = VersionInfo32_Value(info);
+    if ((PBYTE)Ptr >= ((PBYTE)info + info->wLength))
+        Ptr = info->szKey + wcslen(info->szKey);
+
+    *lplpBuffer = Ptr;
     if (puLen)
         *puLen = info->wValueLength;
     if (pbText)
diff --git a/dlls/version/tests/info.c b/dlls/version/tests/info.c
index dcde1bf096f..4472d901f18 100644
--- a/dlls/version/tests/info.c
+++ b/dlls/version/tests/info.c
@@ -701,7 +701,6 @@ static void test_VerQueryValue_EmptyData(void)
     ret = VerQueryValueW(ver, L"\StringFileInfo\FFFF0000\FileVersion", (LPVOID *)&p, &len);
     ok(ret, "VerQueryValueW error %u\n", GetLastError());
     ok(len == 0, "VerQueryValueW returned %u, expected 0\n", len);
-    todo_wine
     ok(p == (ver + offsetof(rsrc_section_t, FileVersion_key) + 11 * sizeof(WCHAR)), "p was %p, expected %p\n", p, ver + offsetof(rsrc_section_t, FileVersion_key) + 11 * sizeof(WCHAR));

/* The key behind it, to show that parsing continues just fine */

[PATCH 2/2] kernelbase: Fix VerQueryValueW with no data

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125779

Your paranoid android.

=== debian11 (32 bit report) ===

httpapi:
Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x7b020c3f).