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

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



user ssh@packages.debian.org
usertag 392669 not-a-bug
thanks

> Package: openssh-server
> Version: 1:4.3p2-5
> Severity: normal
> File: /usr/sbin/sshd
>
> Hello,
>
> recently I discovered the following bit of code in the source file
> "openssh-4.3p2/openbsd-compat/xmmap.c" (function 'xmmap', around line
> 51):
>
>     #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.  Actually, gcc -W -Wall will warn if you
initialize a character array with a string greater than can be stored
there.

I'll note two things; the above code could be written more simply,
succinctly, and, IMO, more cleanly as:

  char tmpname[] = MM_SWAP_TEMPLATE;

which avoid the issue entirely.

Also, gcc -W -Wall seems to have an off by one of its own!  Although I
don't have account access to retest right now, I recall that

  char foo[1]="bar";

did not cause a warning, but

  puts(foo);

printed garbage for some cases when gcc -W -Wall did not warn.  Please
test this, or cluebat me if I'm wrong.

Justin





Reply to: