This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Bug? wcsxfrm causing memory corruption


On Tue, Jun 6, 2017 at 9:22 PM, Corinna Vinschen wrote:
> On Jun  6 17:57, Corinna Vinschen 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...
>> > >> [...]
>> 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
>
> Latest snapshot from https://cygwin.com/snapshots/ has the patch applied.
> Please try.

I had a look at the fix and it looks good to me; thanks!

Erik

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]