Bug? wcsxfrm causing memory corruption

Tue Jun 6 15:57:00 GMT 2017

Hi Erik,
On May 22 14:50, Erik Bray wrote:
> On Sun, May 21, 2017 at 6:23 AM, Duncan Roe wrote:
> > On Wed, May 10, 2017 at 11:30:46AM +0200, Erik Bray wrote:
> >> [...]
> >> The attached test demonstrates the bug. Given an output buffer of N
> >> wide characters, wcsxfrm will cause bytes beyond the destination size
> >> to be reversed. I believe it might actually be a bug in the underlying
> >> LCMapStringW workhorse (this is on Windows 10; have not tested other
> >> versions).
> >>
> >> According to its docs [1], the cchDest argument (size of the
> >> destination buffer) is treated as a *byte* count when using
> >> LCMAP_SORTKEY. However, for the purposes of applying the
> >> LCMAP_BYTEREV transformation it seems to be treating the output size
> >> (in bytes) as character count. So in the example I give, where the
> >> output sort key is 7 bytes (including the null terminator), it swaps
> >> *14* bytes--the bytes including the sort key as well as the next 7
> >> adjacent bytes. This is obviously a problem if the destination buffer
> >> is allocated out of some larger memory pool.
> >>
> >> This definitely has to be a bug, right? Or at least very poorly
> >> documented on MS's part. A workaround would either be to not use
> >> LCMAP_BYTEREV and just swap the bytes manually, or in a second call to
> >> LCMapStringW with LCMAP_BYTEREV and the correct character count...
> >> [...]
> > Hi Erik,
> >
> > I get
> > [...]
> Yes, that's the same. Thanks for giving it a try--I should have
> included example output in my original message.
>> You can see in the last case that without LCMAP_BYTEREV it writes the sequence
>> 0x0e 0x09 0x01 0x01 0x01 0x01 0x00
>> with a terminating 0x00. Bytes after that remain unchanged. In the
> other two examples *with* LCMAP_BYTEREV, the terminating 0x00 gets
> swapped with the 0x07 after it, but this documented and expected
> behavior of LCMapStringW, and is already accounted for in Cygwin's
> wcsxfrm. What is undocumented, and unexpected, is that it also byte
> swaps 3 more byte pairs after the actual sort key, which can corrupt
> memory unexpectedly.

Incredible piece of detective work. Yeah, this looks very much like a
bug in LCMapStringW, embarrassingly one I didn't notice at the time of
writing the code. I just tested this on W7 and the problem was already
present.
I don't know if this could be fixed by documentation. There's just no
safe way to combine LCMAP_SORTKEY with LCMAP_BYTEREV it seems, unless
you always allocate a buffer at least twice as big as actually required
for the sort key. 
So, yeah, I think manual swaping after calling LCMapStringW without
LCMAP_BYTEREV is the way to go. I'll prepare a fix.
Thanks for tracking down and the testcase,
Corinna
-- 
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin/attachments/20170606/d4869d5d/attachment.sig>