Re: Fix writing beyond array bounds in codecvt/unshift/char/1.cc

[Jonathan Wakely <jwakely dot gcc at gmail dot com>]

On 22 April 2010 15:41, Alexander Monakov wrote:
>
>
> On Thu, 22 Apr 2010, Jonathan Wakely wrote:
>
>> On 22 April 2010 15:12, Alexander Monakov wrote:
>> >> But now when c_lit+size is passed to cvt->in() and cvt->out() it will
>> >> be too large.
>> >
>> > Sorry, I do not agree. ?Those are past-the-end iterators used to define a
>> > sequence [c_lit, c_lit+size) (right end not inclusive) and must be legal.
>> > Or am I missing something?
>>
>> Yes, you're missing that c_lit+strlen(c_lit) is already past-the-end,
>> i.e. it points to the nul terminator in the last element of c_lit.
>>
>> If you pass c_lit+strlen(c_lit)+1 then that is one ?past the nul
>> terminator, and so codecvt::in will try to read the nul, which is not
>> necessary.
>
> OK, thanks for the explanation. ?I'll change str{cmp,cpy} to the corresponding
> mem* functions then. ?Is the following OK?
>
> 2010-04-22 ?Alexander Monakov ?<amonakov@ispras.ru>
>
> ? ? ? ?* 22_locale/codecvt/unshift/char/1.c (test01): Clarify size definition.
> ? ? ? ?Use memcpy and memcmp to avoid access beyond allocated memory.
>
> diff --git a/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc b/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> index ba417af..0fa7a13 100644
> --- a/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> +++ b/libstdc++-v3/testsuite/22_locale/codecvt/unshift/char/1.cc
> @@ -35,7 +35,7 @@ void test01()
> ? bool test __attribute__((unused)) = true;
> ? const char* ? ? ? ? ?c_lit = "black pearl jasmine tea";
> ? const char* ? ? ? ? ?from_next;
> - ?int ? ? ? ? ? ? ? ? ?size = 23;
> + ?int ? ? ? ? ? ? ? ? ?size = strlen(c_lit);
> ? char* ? ? ? ? ? ? ? ?c_arr = new char[size];
> ? char* ? ? ? ? ? ? ? ? c_ref = new char[size];
> ? char* ? ? ? ? ? ? ? ? ? ? ? ?to_next;
> @@ -68,10 +68,10 @@ void test01()
> ? VERIFY( to_next == c_arr );
>
> ? // unshift
> - ?strcpy(c_arr, c_lit);
> + ?memcpy(c_arr, c_lit, size);
> ? result r3 = cvt->unshift(state, c_arr, c_arr + size, to_next);
> ? VERIFY( r3 == codecvt_base::noconv );
> - ?VERIFY( !strcmp(c_arr, c_lit) );
> + ?VERIFY( !memcmp(c_arr, c_lit, size) );
> ? VERIFY( to_next == c_arr );
>
> ? delete [] c_arr;
>

No, it should be std::strlen because it includes <cstring> not <string.h>

I still think allocating size+1is better, but won't object if you
insist on changing strcmp to memcmp