Re: ssh-1.2.26 buffer overflow patch

[Michael_Jennings () mw 3com com (Michael Jennings)]

On Monday, 02 November 1998, at 02:32:50 (+0000),
Andy Church wrote:

>      For those with OS's that have snprintf() in libc (I think this
> includes at least Linux and FreeBSD--I don't know about others), the patch
> below fixes the overflows mentioned in the recent Rootshell advisory about
> ssh.  If you're on a system that doesn't have snprintf(), you can probably
> do something with support/vsnprintf.c from wu-ftpd if you want to avoid the
> stupid licensing restrictions on ssh 2.0.x.
>
>      And as a comment on SSH Communications' damage-control statement of
> "[n]o buffer overflows nor any other security bugs were found":
>
> [login.c:538]
> log_msg("putuserattr S_LASTTTY %.900s failed: %.100s", /*...*/);
>
> [log-server.c:130]
> void log_msg(const char *fmt, ...)
> {
>   char buf[1024];
>   /*...*/
>   vsprintf(buf, fmt, args);
>   /*...*/
> }
>
> Granted, I haven't checked whether this is exploitable, but I could never
> call that secure.  (Count the bytes.)

Note lines 538-539 of login.c are:

        log_msg("putuserattr S_LASTTTY %.900s failed: %.100s",
            ttyname, strerror(errno));

Also note that ttyname is generated by pty_allocate() at line 101 of
pty.c from system calls on *all* systems.  (It is the "namebuf"
parameter.)  And further note that ttyname is declared as a static
buffer of 64 bytes at sshd.c:2732.

Considering the fact that we're talking about both these parameters
being system generated, and the first of which rarely if ever exceeds
15 or so characters, and the second of which rarely if ever exceeds 40
or so characters, I'd say it's pretty safe.  Add that to the fact that
overflowing log_msg's buf would require quite a few more than 64 bytes
in ttyname, which would overflow do_authenticated()'s ttyname buffer
long before reaching the log_msg() call.  (record_login() is called
from do_exec_pty(), which do_authenticated() calls long after
allocating the pty in pty_allocate(), which is where the overflow
would have to occur.)

Michael

--
 "I've been looking for a Savior in these dirty streets,
  Looking for a Savior beneath these dirty sheets."
                                               -- Tori Amos, "Crucify"
=======================================================================
Michael Jennings        <mej () mw 3com com>        http://www.tcserv.com/
Senior Systems Engineer                 3Com Global Information Systems