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

[Mark Jansen]

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  wrote:
> 2015-02-13 20:41 GMT+01:00 Mark Jansen :
> > 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
> >  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-----