Possible buffer overflow in TFTP client?

["Karl Vogel" <vogelke@dnaco.net>]

Greetings:

I think I found a buffer overflow in the TFTP client:
http://download.sourceforge.net/mirrors/OpenBSD/src/usr.bin/tftp/main.c

    ...
    52  /*
    53   * TFTP User Program -- Command Interface.
    54   */
    55  #include <sys/types.h>
    56  #include <sys/socket.h>
    57  #include <sys/file.h>
    58  
    ...
    84  char    mode[32];
    85  char    line[LBUFLEN];
    86  int     margc;
    87  char    *margv[20];
    88  char    *prompt = "tftp";
    ...

margv has a maximum of 20 slots.  I saw no checking code in makeargv() or
the routines that call it:

   193  void
   194  setpeer(argc, argv)
   195      int argc;
   196      char *argv[];
   197  {
   198      struct hostent *host;
   199  
   200      if (argc < 2) {
   201          strcpy(line, "Connect ");
   202          printf("(to) ");
   203          fgets(&line[strlen(line)], LBUFLEN-strlen(line), stdin);
   204          makeargv();
   205          argc = margc;
   206          argv = margv;
   207      }
   ...

   658  /*
   659   * Slice a string up into argc/argv.
   660   */
   661  static void
   662  makeargv()
   663  {
   664      register char *cp;
   665      register char **argp = margv;
   666  
   667      margc = 0;
   668      for (cp = line; *cp;) {
   669          while (isspace(*cp))
   670              cp++;
   671          if (*cp == '\0')
   672              break;
   673          *argp++ = cp;
   674          margc += 1;
   675          while (*cp != '\0' && !isspace(*cp))
   676              cp++;
   677          if (*cp == '\0')
   678              break;
   679          *cp++ = '\0';
   680      }
   681      *argp++ = 0;
   682  }
   ...

What if someone passed in a line with more than 20 arguments?  One possible
fix wouldn't be too tough.  Add a macro near 57:

    56  #include <sys/socket.h>
    57  #include <sys/file.h>
        #define MAXARGV 20

Change 87:

    86  int     margc;
    87  char    *margv[MAXARGV+1];
    88  char    *prompt = "tftp";

Add a check at the top of the loop in makeargv():

   667      margc = 0;
   668      for (cp = line; *cp;) {
                if (margc >= MAXARGV)
                    break;
   669          while (isspace(*cp))

-- 
Karl Vogel                <vogelke@dnaco.net>
ASC/YCOA, Wright-Patterson AFB, OH 45433, USA

Most of us spend the first 6 days of each week sowing wild oats, then we go
to church on Sunday and pray for a crop failure.  --Fred Allen