Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dfc33583-c665-05cd-9847-9c06c4708c8c@gmail.com>
Date: Wed, 9 Aug 2017 20:57:27 +0300
From: Mikhail Kremnyov <mkremnyov@...il.com>
To: musl@...ts.openwall.com
Subject: Re: Issues in mbsnrtowcs and wcsnrtombs

Hi,

Here are two patches, one (for libc_test) adds a couple of tests to
reproduce the issues, the other one fixes them.

Mikhail.

On 07/18/2017 11:05 PM, Mikhail Kremnyov wrote:
> Hi,
>
> It looks like there are some bugs in the implementations of mbsnrtowcs
> and wcsnrtombs.
> E.g. inside mbsnrtowcs there is this code:
>
>     while ( s && wn && ( (n2=n/4)>=wn || n2>32 ) ) {
>         if (n2>=wn) n2=wn;
>         n -= n2;
>         l = mbsrtowcs(ws, &s, n2, st);
>
> Here "n" is the number of source bytes to convert and "n2" is the number
> of wide chars that may be put to the destination, so it's incorrect to
> subtract one from another. And indeed a simple test shows that the
> function doesn't work correctly if long enough non-ascii string is
> passed to it. E.g.:
>
>     const std::string origStr =
> u8"абвгдеёжзийклмнопрстуфхцчшщъыьэюяАБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯ";
>     const std::string srcStr = origStr + u8"їґіє";
>
>     std::mbstate_t st = {};
>     const char* srcPtr = &srcStr[0];
>     std::wstring dest(srcStr.length() + 1, wchar_t(0));
>
>     auto res = mbsnrtowcs(&dest[0], &srcPtr, origStr.length(),
> dest.length(), &st);
>
>     std::cout << "res = " << res << ", srcPtr = " << (void*)srcPtr <<
> std::endl;
>
> And the output is:
>     res = 70, srcPtr = 0
>
> Here mbsnrtowcs was told to convert only "origStr.length()" number of
> bytes, which contain 66 2-byte characters, but it converted 70, stopping
> only after the zero char was met.
>
> A similar problem happens with wcsnrtombs using a slightly longer string:
>
>     std::wstring srcStr =
> L"абвгдеёжзийклмнопрстуфхцчшщъыьэюяАБВГДЕЁЖЗИЙКЛМНОПРСТУФХЦЧШЩЪЫЬЭЮЯабвгдеёжзийклмнопрстуфхцчшщъыьэюя";
>
>     const wchar_t* srcPtr = &srcStr[0];
>     std::mbstate_t st = {};
>     std::string dest(srcStr.length() * 4 + 1, char(0));
>
>     auto res = wcsnrtombs(&dest[0], &srcPtr, srcStr.length(),
> dest.length(), &st);
>
>     std::cout << "res = " << res << ", dest = " << dest << std::endl;
>
> The output:
>     res = 98, dest = абвгдеёжзийклмнопрстуфхцчшщъыьэюяАБВГДЕЁЖЗИЙКЛМНО
>    
> I.e. it only converted 49 characters instead of 99.
>
>
> Mikhail.
>
>


View attachment "libc_test.diff" of type "text/x-patch" (2725 bytes)

View attachment "musl.diff" of type "text/x-patch" (1232 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.