bin/118001: sysinstall can't read some packages from INDEX. (buffer overflow).

[Brad Hall]

The following reply was made to PR bin/118001; it has been noted by GNATS.

From: Brad Hall <haws at ethereal.net>
To: bug-followup at FreeBSD.org, llevinson at mail.ru
Cc:  
Subject: Re: bin/118001: sysinstall can't read some packages from INDEX. (buffer overflow).
Date: Tue, 11 Dec 2007 14:52:44 -0800

 Hi,
 
 I just saw this in the 7.0BETA4 installer.  A better fix may be to just
 advance the pointer to the next sep insetad of copying the not used
 stuff into a junk buffer (since copy_to_sep doesn't check the size of
 its buffer).  Here is a patch:
 
 Index: index.c
 ===================================================================
 RCS file: /home/ncvs/src/usr.sbin/sysinstall/index.c,v
 retrieving revision 1.115
 diff -d -u -r1.115 index.c
 --- index.c     28 Jun 2007 17:42:20 -0000      1.115
 +++ index.c     11 Dec 2007 22:43:10 -0000
 @@ -270,6 +270,19 @@
  }
  
  static int
 +advance_to_sep(char *from, int sep)
 +{
 +    char *tok;
 +
 +    tok = strchr(from, sep);
 +    if (!tok) {
 +       return 0;
 +    }
 +    *tok = '\0';
 +    return tok + 1 - from;
 +}
 +
 +static int
  readline(FILE *fp, char *buf, int max)
  {
      int rv, i = 0;
 @@ -291,11 +304,14 @@
  index_parse(FILE *fp, char *name, char *pathto, char *prefix, char *comment, char *descr, char *maint, char *cats, char *rdeps, int *volume)
  {
      char line[10240 + 2048 * 7];
 -    char junk[2048];
      char volstr[2048];
      char *cp;
      int i;
  
 +    /*
 +     * NOTE: Just advance to the separator for fields that are
 +     * not used instead of copying them into a junk buffer
 +     */
      i = readline(fp, line, sizeof line);
      if (i <= 0)
         return EOF;
 @@ -307,21 +323,20 @@
      cp += copy_to_sep(descr, cp, '|');         /* path to pkg-descr */
      cp += copy_to_sep(maint, cp, '|');         /* maintainer */
      cp += copy_to_sep(cats, cp, '|');          /* categories */
 -    cp += copy_to_sep(junk, cp, '|');          /* build deps - not used */
 +    cp += advance_to_sep(cp, '|');             /* build deps - not used */
      cp += copy_to_sep(rdeps, cp, '|');         /* run deps */
      if (index(cp, '|'))
 -        cp += copy_to_sep(junk, cp, '|');      /* url - not used */
 +        cp += advance_to_sep(cp, '|');         /* url - not used */
      else {
 -       strncpy(junk, cp, 1023);
         *volume = 0;
         return 0;
      }
      if (index(cp, '|'))
 -       cp += copy_to_sep(junk, cp, '|');       /* extract deps - not used */
 +       cp += advance_to_sep(cp, '|');          /* extract deps - not used */
      if (index(cp, '|'))
 -       cp += copy_to_sep(junk, cp, '|');       /* patch deps - not used */
 +       cp += advance_to_sep(cp, '|');          /* patch deps - not used */
      if (index(cp, '|'))
 -       cp += copy_to_sep(junk, cp, '|');       /* fetch deps - not used */
 +       cp += advance_to_sep(cp, '|');          /* fetch deps - not used */
      if (index(cp, '|'))
          cp += copy_to_sep(volstr, cp, '|');    /* media volume */
      else {
 
 
 Thanks,
 Brad