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

[jzbiciak () DALDD SC TI COM (Joe Zbiciak)]

'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>--------+