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

[Matteo Bruni]

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-----