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 14:14, Alexander Monakov wrote:
> [adding libstdc++@gcc.gnu.org to Cc:]
>
> Alexander Monakov <amonakov@ispras.ru> writes:
>> > 'size' definition seemingly does not account for a zero byte, and thus strcpy
>> > call near the end of the test overwrites one byte beyond allocated memory,
>> > which may lead to spurious test failures. ?The patch simply bumps size to 24.
>
> On Tue, 13 Apr 2010, Andreas Schwab wrote:
>
>> Alexander Monakov <amonakov@ispras.ru> writes:
>>
>> > 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..8a45efc 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 = 24;
>>
>> How about using strlen(c_lit)?
>>
>> > ? ?char* ? ? ? ? ? ?c_arr = new char[size];
>> > ? ?char* ? ? ? ? ? ? ? ? c_ref = new char[size];
>>
>> I think you rather want to use size+1 for the allocations.
>>
>> Andreas.
>>
>>
>
> I'd rather keep allocations as is, since size is passed as an argument into
> various functions in the test, and we probably want to notice them access one
> element beyond array bounds. ?Thus, I'm changing size to strlen(c_lit) + 1.
>
> OK to commit?
>
> 2010-04-22 ?Alexander Monakov ?<amonakov@ispras.ru>
>
> ? ? ? ?* 22_locale/codecvt/unshift/char/1.cc (test01): Correct definition of size.
>
> 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..6e851ff 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) + 1;
> ? char* ? ? ? ? ? ? ? ?c_arr = new char[size];
> ? char* ? ? ? ? ? ? ? ? c_ref = new char[size];
> ? char* ? ? ? ? ? ? ? ? ? ? ? ?to_next;

But now when c_lit+size is passed to cvt->in() and cvt->out() it will
be too large.

I agree with Andreas, allocate size+1, do not change size.

Alternatively, do something like change strcpy to memcpy, so it
doesn't write a NUL.