[PATCH v3 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

--
  v3: kernelbase: Fix VerQueryValueW with no data

https://gitlab.winehq.org/wine/wine/-/merge_requests/1263

[PATCH v3 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..60c5b208993 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 %lu\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 %lu\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 v3 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..394a8a4f1c7 100644
--- a/dlls/kernelbase/version.c
+++ b/dlls/kernelbase/version.c
@@ -1071,6 +1071,7 @@ static BOOL VersionInfo16_QueryValue( const VS_VERSION_INFO_STRUCT16 *info, LPCS
 static BOOL VersionInfo32_QueryValue( const VS_VERSION_INFO_STRUCT32 *info, LPCWSTR lpSubBlock,
                                       LPVOID *lplpBuffer, UINT *puLen, BOOL *pbText )
 {
+    PVOID Ptr;
     TRACE("lpSubBlock : (%s)\n", debugstr_w(lpSubBlock));

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 60c5b208993..31d4a8d11dc 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 %lu\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 */