From owner-freebsd-audit Fri Aug 4 21: 3:56 2000 Delivered-To: freebsd-audit@freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21]) by hub.freebsd.org (Postfix) with ESMTP id 33C1637B8E7 for ; Fri, 4 Aug 2000 21:03:51 -0700 (PDT) (envelope-from kris@FreeBSD.org) Received: from localhost (kris@localhost) by freefall.freebsd.org (8.9.3/8.9.2) with ESMTP id VAA65322 for ; Fri, 4 Aug 2000 21:03:50 -0700 (PDT) (envelope-from kris@FreeBSD.org) X-Authentication-Warning: freefall.freebsd.org: kris owned process doing -bs Date: Fri, 4 Aug 2000 21:03:50 -0700 (PDT) From: Kris Kennaway To: audit@freebsd.org Subject: ftp(1) patch Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Bunch of overflows here..I'm slightly unsure of the macro expansion one - can everyone please check it? Incidentally, there was a case of strncpy() abuse here (using strlen as the third argument) Kris Index: cmds.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ftp/cmds.c,v retrieving revision 1.18 diff -u -r1.18 cmds.c --- cmds.c 2000/06/24 15:34:30 1.18 +++ cmds.c 2000/08/05 03:52:38 @@ -125,7 +125,7 @@ else comret = command("TYPE %s", p->t_mode); if (comret == COMPLETE) { - (void)strcpy(typename, p->t_name); + (void)strlcpy(typename, p->t_name, sizeof(typename)); curtype = type = p->t_type; } } Index: complete.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ftp/complete.c,v retrieving revision 1.5 diff -u -r1.5 complete.c --- complete.c 1999/08/28 01:01:30 1.5 +++ complete.c 2000/08/05 03:34:46 @@ -92,7 +92,7 @@ return (CC_ERROR); /* no choices available */ if (words->sl_cur == 1) { /* only once choice available */ - (void)strcpy(insertstr, words->sl_str[0]); + (void)strlcpy(insertstr, words->sl_str[0], sizeof(insertstr)); if (el_insertstr(el, insertstr + wordlen) == -1) return (CC_ERROR); else @@ -276,7 +276,7 @@ printf("\n%s\n", emesg); return (CC_REDISPLAY); } - (void)strcpy(lastdir, dir); + (void)strlcpy(lastdir, dir, sizeof(lastdir)); dirchange = 0; } Index: domacro.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ftp/domacro.c,v retrieving revision 1.6 diff -u -r1.6 domacro.c --- domacro.c 1999/08/28 01:01:31 1.6 +++ domacro.c 2000/08/05 03:49:13 @@ -56,7 +56,7 @@ int argc; char *argv[]; { - int i, j, count = 2, loopflg = 0; + int i, j, count = 2, linefull = 0, loopflg = 0; char *cp1, *cp2, line2[200]; struct cmd *c; @@ -75,7 +75,7 @@ code = -1; return; } - (void)strcpy(line2, line); + (void)strlcpy(line2, line, sizeof(line2)); TOP: cp1 = macros[i].mac_start; while (cp1 != macros[i].mac_end) { @@ -83,7 +83,7 @@ cp1++; } cp2 = line; - while (*cp1 != '\0') { + while (*cp1 != '\0' && !linefull) { switch(*cp1) { case '\\': *cp2++ = *++cp1; @@ -96,7 +96,12 @@ } cp1--; if (argc - 2 >= j) { - (void)strcpy(cp2, argv[j+1]); + if (strlcpy(cp2, argv[j+1], + sizeof(line) - (cp2 - line)) >= + sizeof(line) - (cp2 - line)) { + linefull = 1; + break; + } cp2 += strlen(argv[j+1]); } break; @@ -105,8 +110,13 @@ loopflg = 1; cp1++; if (count < argc) { - (void)strcpy(cp2, argv[count]); - cp2 += strlen(argv[count]); + if (strlcpy(cp2, argv[count], + sizeof(line) - (cp2 - line)) >= + sizeof(line) - (cp2 - line)) { + linefull = 1; + break; + } + cp2 += strlen(argv[count]); } break; } @@ -141,7 +151,7 @@ if (bell && c->c_bell) { (void)putchar('\007'); } - (void)strcpy(line, line2); + (void)strlcpy(line, line2, sizeof(line)); makeargv(); argc = margc; argv = margv; Index: fetch.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.14 diff -u -r1.14 fetch.c --- fetch.c 2000/06/24 15:34:31 1.14 +++ fetch.c 2000/08/05 03:50:21 @@ -598,7 +598,7 @@ if (strcmp(host, lasthost) != 0) { int oautologin; - (void)strcpy(lasthost, host); + (void)strlcpy(lasthost, host, sizeof(lasthost)); if (connected) disconnect(0, NULL); xargv[0] = __progname; Index: ftp.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ftp/ftp.c,v retrieving revision 1.31 diff -u -r1.31 ftp.c --- ftp.c 2000/06/24 15:34:31 1.31 +++ ftp.c 2000/08/05 03:30:36 @@ -112,11 +112,9 @@ char *host; if (*host0 == '[' && strrchr(host0, ']') != NULL) { /*IPv6 addr in []*/ - strncpy(hostnamebuf, host0 + 1, strlen(host0) - 2); - hostnamebuf[strlen(host0) - 2] = '\0'; + strlcpy(hostnamebuf, host0 + 1, MIN(strlen(host0) - 1, sizeof(hostnamebuf))); } else { - strncpy(hostnamebuf, host0, strlen(host0)); - hostnamebuf[strlen(host0)] = '\0'; + strlcpy(hostnamebuf, host0, MIN(strlen(host0) + 1, sizeof(hostnamebuf))); } host = hostnamebuf; memset(&hints, 0, sizeof(hints)); @@ -135,7 +133,7 @@ res = res0; if (res->ai_canonname) - (void) strncpy(hostnamebuf, res->ai_canonname, + (void) strlcpy(hostnamebuf, res->ai_canonname, sizeof(hostnamebuf)); hostname = hostnamebuf; while (1) { @@ -440,8 +438,7 @@ if (len > sizeof(reply_string)) len = sizeof(reply_string); - (void)strncpy(reply_string, current_line, len); - reply_string[len] = '\0'; + (void)strlcpy(reply_string, current_line, len); } if (continuation && code != originalcode) { if (originalcode == 0) @@ -1658,20 +1655,16 @@ mcase = op->mcse; ip->ntflg = ntflag; ntflag = op->ntflg; - (void)strncpy(ip->nti, ntin, sizeof(ip->nti) - 1); - (ip->nti)[sizeof(ip->nti) - 1] = '\0'; - (void)strcpy(ntin, op->nti); - (void)strncpy(ip->nto, ntout, sizeof(ip->nto) - 1); - (ip->nto)[sizeof(ip->nto) - 1] = '\0'; - (void)strcpy(ntout, op->nto); + (void)strlcpy(ip->nti, ntin, sizeof(ip->nti)); + (void)strlcpy(ntin, op->nti, sizeof(ntin)); + (void)strlcpy(ip->nto, ntout, sizeof(ip->nto)); + (void)strlcpy(ntout, op->nto, sizeof(ntout)); ip->mapflg = mapflag; mapflag = op->mapflg; - (void)strncpy(ip->mi, mapin, sizeof(ip->mi) - 1); - (ip->mi)[sizeof(ip->mi) - 1] = '\0'; - (void)strcpy(mapin, op->mi); - (void)strncpy(ip->mo, mapout, sizeof(ip->mo) - 1); - (ip->mo)[sizeof(ip->mo) - 1] = '\0'; - (void)strcpy(mapout, op->mo); + (void)strlcpy(ip->mi, mapin, sizeof(ip->mi)); + (void)strlcpy(mapin, op->mi, sizeof(mapin)); + (void)strlcpy(ip->mo, mapout, sizeof(ip->mo)); + (void)strlcpy(mapout, op->mo, sizeof(mapout)); (void)signal(SIGINT, oldintr); if (abrtflag) { abrtflag = 0; @@ -1862,14 +1855,11 @@ warn("local: %s", local); return ((char *) 0); } - (void)strcpy(new, local); + (void)strlcpy(new, local, sizeof(new)); cp = new + strlen(new); - *cp++ = '.'; - while (!d) { - if (++count == 100) { - puts("runique: can't find unique file name."); - return ((char *) 0); - } + if (cp - new < sizeof(new)) + *cp++ = '.'; + while (!d && ++count < 100 && cp - new < sizeof(new)) { *cp++ = ext; *cp = '\0'; if (ext == '9') @@ -1886,6 +1876,10 @@ *(cp - 2) = *(cp - 2) + 1; cp--; } + } + if (count == 100 || cp - new >= sizeof(new)) { + puts("runique: can't find unique file name."); + return ((char *) 0); } return (new); } Index: main.c =================================================================== RCS file: /home/ncvs/src/usr.bin/ftp/main.c,v retrieving revision 1.28 diff -u -r1.28 main.c --- main.c 2000/06/24 15:34:31 1.28 +++ main.c 2000/08/05 03:57:59 @@ -258,7 +258,7 @@ pw = getpwuid(getuid()); if (pw != NULL) { home = homedir; - (void)strcpy(home, pw->pw_dir); + (void)strlcpy(home, pw->pw_dir, sizeof(homedir)); } setttywidth(0); -- In God we Trust -- all others must submit an X.509 certificate. -- Charles Forsythe To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message