Vulnerability Development mailing list archives

openssh vulnerability


From: Diode Trnasistor <ffddfe () yahoo com>
Date: Tue, 16 Sep 2003 07:49:33 -0700 (PDT)

In case you haven't been following it, on full
disclosure there's been some mention of a new ssh
vulnerability.

The vulnerability is allegedly in the following
sniplet of code taken from buffer.c file of openssh
distrib:

void *
buffer_append_space(Buffer *buffer, u_int len)
{
  void *p;
  if (len > 0x100000)
  fatal("buffer_append_space: len %u not 
         supported",len);

  /* If the buffer is empty, start using it from the 
     beginning. */
  if (buffer->offset == buffer->end) {
    buffer->offset = 0;
    buffer->end = 0;
  }
restart:
/* If there is enough space to store all data, store 
   it now. */
  if (buffer->end + len < buffer->alloc) {
    p = buffer->buf + buffer->end;
    buffer->end += len;
    return p;
  }
/*
 * If the buffer is quite empty, but all data is at   
   the end, move the
* data to the beginning and retry.
 */

  if (buffer->offset > buffer->alloc / 2) {
    memmove(buffer->buf, buffer->buf + buffer->offset,
    buffer->end - buffer->offset);
    buffer->end -= buffer->offset;
    buffer->offset = 0;
    goto restart;
   }
/* Increase the size of the buffer and retry. */
  buffer->alloc += len + 32768;
  if (buffer->alloc > 0xa00000)
    fatal("buffer_append_space: alloc %u not 
           supported", buffer->alloc);
  buffer->buf = xrealloc(buffer->buf, buffer->alloc);
  goto restart;
 /* NOTREACHED */
}

The structure of Buffer struct is:
typedef struct {
        u_char  *buf; /* Buffer for data. */
        u_int    alloc; /* Number of bytes allocd */
        u_int    offset; /* Offset of first byte 
                            containing data. */
        u_int    end;   /* Offset of last byte 
                           containing data. */
}       Buffer;


Now reading the code i can't seem to find anything
wrong, but there is already a patch out!  Patch is at:
http://www.freebsd.org/cgi/cvsweb.cgi/src/crypto/openssh/buffer.c.diff?r1=1.1.1.6&r2=1.1.1.7&f=h

Looking at patch they added a variable
u_int newlen;

And then at the very bottom where the xrealloc happens
newlen = buffer->alloc + len + 32768;

and finally:
buffer->buf = xrealloc(buffer->buf, newlen);
buffer->alloc = newlen;

So all that's accomplished is not using the value
inside the buffer to be reallocated as the number of
bytes to allocate.  It is now done with a new
variable.

Is anyone familiar with what happens when you use
realloc like they are using originally (when using a
value instead the structure to reallocate as the
second value to realloc).  I still fail to see how
this is a security problem, and would like it if
someone would explain it to me.  Thanx :)

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com


Current thread: