Re: A couple of patches (RFC931 and scp location)

Joe Zbiciak (jzbiciak@DALDD.SC.TI.COM)
Mon, 09 Jun 1997 18:42:51 -0500

'Matt Simmons' said previously:
|
| I added the authuser code from wu-ftpd 2.4 to ssh, and mentioned it in
| a post to the ssh list. One of the subscribers to that list, Benjamin
| Stassart, looked through it and found a possible buffer overrun.

This does not appear to be a buffer overrun condition. Here's my analysis
of this code:

/* Read a single character */
> while ((w = read(s, &ch, 1)) == 1) {

/* Assign that to a (valid) position in the buffer. */
> *buf = ch;

/* If that character isn't whitespace, step along the buffer */
> if ((ch != ' ') && (ch != '\t') && (ch != '\r'))
> ++buf;

/* If we've reached the end of the buffer, or see a newline
terminate the loop. */
> if ((buf - realbuf == sizeof(realbuf) - 1) || (ch == '\n'))
> break;
> }

At no point is a character assigned to a position outside the buffer.
All whitespace characters (except newlines) are thrown away by this
function, it appears.

I make the tacit assumption that "buf" is a valid pointer into "realbuf".
If that's not the case, then please let me know. Otherwise, am I missing
something here?

If there is no guarantee prior to this loop that "buf" is valid, then
the loop should be rewritten like so:

while ((buf - realbuf < sizeof(realbuf) - 1) &&
(w = read(s, &ch, 1)) == 1) {
*buf = ch;
if ((ch != ' ') && (ch != '\t') && (ch != '\r'))
++buf;
if (ch == '\n')
break;
}

Regards,

--Joe

--
 +--------------Joseph Zbiciak--------------+
 |- - - - jzbiciak@daldd.sc.ti.com - - - - -|
 | - - http://www.primenet.com/~im14u2c - - |      Not your average "Joe."
 |- - - - Texas Instruments,  Dallas - - - -|
 +-------#include <std_disclaimer.h>--------+