[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#392669: /usr/sbin/sshd: off-by-one error in function 'xmmap'



> Hi Justin,
>
> On Fri, Oct 13, 2006 at 07:01:51PM -0400, Justin Pryzby wrote:
>> >     #define MM_SWAP_TEMPLATE "/var/run/sshd.mm.XXXXXXXX"
>> >         if (address == (void *)MAP_FAILED) {
>> >                 char tmpname[sizeof(MM_SWAP_TEMPLATE)] =
>> MM_SWAP_TEMPLATE;
>> >                 int tmpfd;
>> >                 mode_t old_umask;
>> >
>> >                 old_umask = umask(0177);
>> >                 tmpfd = mkstemp(tmpname);
>> >
>> > The array 'tmpname' has enough space to contain the string
>> > MM_SWAP_TEMPLATE, but not the terminating '\0' byte.  Therefore
>> > 'mkstemp' is called with an unterminated string.
>> In a test, a string constant has sizeof(s)==1+strlen(s); this happens
>> for
>> a character constant, too.  I'm assuming that gcc implements the
>> behavior
>> required by relevant standards.
>
> Sorry, I can't quite follow you here.  Is your claim that
>
>     char tmpname[sizeof(MM_SWAP_TEMPLATE)] = MM_SWAP_TEMPLATE;
>
> will make 'tmpname' long enough to contain the trailing '\0'?
Correct.

>  And what do you mean by "relevant standards"?
Whatever antique and modern standards define "The C Programming Language".

> The problem, reduced to a simpler form, is shown in the following
> example:
>
>     voss@burmah [~] cat t.c
>     #include <stdio.h>
>     int
>     main()
>     {
>       char string[4]="1234";
>       puts(string);
>       return 0;
>     }
>     voss@burmah [~] gcc t.c
>     voss@burmah [~] ./a.out
>     1234@o?=o?
> Clearly 'string' was not terminated there.  Why do you think that the
> original issue is different?
Whether you use

  #define FOO "1234"

or

  char foo[]="1234";

strlen(foo) = 4 (does not include the null byte); and,
sizeof(foo) = 5 (includes the null byte).

Your example doesn't use sizeof; try sizeof("1234"), which, it is my
recollection, will evaluate to 5.

Justin





Reply to: