[Bug 24159] Windows Sysinternals 'sigcheck -a' outputs garbage for 'WoW.exe' version info 'comments' (version info 'null' values need special treatment)

https://bugs.winehq.org/show_bug.cgi?id=24159

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
            Summary|sigcheck -a outputs garbage |Windows Sysinternals
                   |in comments for WoW.exe     |'sigcheck -a' outputs
                   |                            |garbage for 'WoW.exe'
                   |                            |version info 'comments'
                   |                            |(version info 'null' values
                   |                            |need special treatment)

--- Comment #4 from Anastasius Focht  ---
Hello folks,

confirming.

The 'wow.exe' binary can be downloaded here (no need for full game install):

http://filebeam.com/4fb29bad4de4f6d82f2ea26538335a80

Version info resource dumped with 'Resource hacker' tool:

--- snip ---
1 VERSIONINFO
FILEVERSION 3,3,5,12340
PRODUCTVERSION 3,3,0,0
FILEOS 0x4
FILETYPE 0x1
{
BLOCK "StringFileInfo"
{
    BLOCK "000004b0"
    {
        VALUE "Comments", ""
        VALUE "CompanyName", "Blizzard Entertainment"
        VALUE "FileDescription", "World of Warcraft Retail"
        VALUE "FileVersion", "3, 3, 5, 12340"
        VALUE "InternalName", "World of Warcraft"
        VALUE "LegalCopyright", "Copyright © 2004"
        VALUE "LegalTrademarks", ""
        VALUE "OriginalFilename", "WoW.exe"
        VALUE "PrivateBuild", ""
        VALUE "ProductName", "World of Warcraft"
        VALUE "ProductVersion", "Version 3.3"
        VALUE "SpecialBuild", ""
    }
}

BLOCK "VarFileInfo"
{
    VALUE "Translation", 0x0000 0x04B0
}
}
--- snip ---

Source:
http://source.winehq.org/git/wine.git/blob/05b3d7b69bf183ef879dc42bd11c65923...

--- snip ---
566 typedef struct
567 {
568     WORD wLength;
569     WORD wValueLength;
570     WORD wType; /* 1:Text, 0:Binary */
571     WCHAR szKey[1];
572 #if 0 /* variable length structure */
573     /* DWORD aligned */
574 BYTE Value[];
575     /* DWORD aligned */
576     VS_VERSION_INFO_STRUCT32 Children[];
577 #endif
578 } VS_VERSION_INFO_STRUCT32;
579
580 #define VersionInfoIs16( ver ) \
581     ( ((const VS_VERSION_INFO_STRUCT16 *)ver)->szKey[0] >= ' ' )
582
583 #define DWORD_ALIGN( base, ptr ) \
584     ( (LPBYTE)(base) + ((((LPBYTE)(ptr) - (LPBYTE)(base)) + 3) & ~3) )
585
586 #define VersionInfo16_Value( ver ) \
587     DWORD_ALIGN( (ver), (ver)->szKey + strlen((ver)->szKey) + 1 )
588 #define VersionInfo32_Value( ver ) \
589     DWORD_ALIGN( (ver), (ver)->szKey + strlenW((ver)->szKey) + 1 )
--- snip ---

Annotated memory dump of 'comments' block and follow up (returned by
'VersionInfo32_FindChild'):

--- snip ---
004622C0  00000018  .... ; wLength = 0x18, wValueLength=0
004622C4  00430001  ..C. ; wType=1 (text), szKey[]
004622C8  006D006F  o.m.
004622CC  0065006D  m.e.
004622D0  0074006E  n.t.
004622D4  00000073  s...
004622D8  0017004E  N... ; wLength = 0x4E, wValueLength=0x17
004622DC  00430001  ..C. ; wType=1 (text), szKey[]
004622E0  006D006F  o.m.
004622E4  00610070  p.a.
004622E8  0079006E  n.y.
004622EC  0061004E  N.a.
004622F0  0065006D  m.e.
004622F4  00000000  ....
004622F8  006C0042  B.l.
004622FC  007A0069  i.z.
00462300  0061007A  z.a.
00462304  00640072  r.d.
00462308  00450020   .E.
0046230C  0074006E  n.t.
00462310  00720065  e.r.
00462314  00610074  t.a.
00462318  006E0069  i.n.
0046231C  0065006D  m.e.
00462320  0074006E  n.t.
00462324  00000000  ....
...
--- snip ---

Source:
http://source.winehq.org/git/wine.git/blob/05b3d7b69bf183ef879dc42bd11c65923...

Wine returns 0x004622D8 in *lplpBuffer which is the start of next block.
The application tries to stringify it, causing the garbage printout - even with
'info->wValueLength == 0'.

It seems "null" values are a special case and need to be treated differently.

In order to have the app see a single wide-char null terminator you need to
check if the value address returned from 'VersionInfo32_Value' is outside of
the current block (which is the case here) and instead return a pointer to
either the null terminator of the key string -> 0x004622D6 or to 'wValueLength'
-> 0x004622C2 (which could also serve as wide-char null "replacement" because
it's WORD type and has zero value).

Yes, both would violate the 32-bit alignment constraint for values but I don't
see other ways here to have the app looking at a null terminator which is
located within resource block boundaries.

$ sha1sum Sigcheck.zip 
9d2e414bc5d71a9d6162fb955474c60cc9086c03  Sigcheck.zip

$ du -sh Sigcheck.zip 
172K    Sigcheck.zip

$ wine --version
wine-1.7.23-2-g69e95ac

Regards