From owner-p4-projects@FreeBSD.ORG Sun Jul 15 06:14:18 2007 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 14CF616A403; Sun, 15 Jul 2007 06:14:18 +0000 (UTC) X-Original-To: perforce@FreeBSD.org Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id BB52816A400 for ; Sun, 15 Jul 2007 06:14:17 +0000 (UTC) (envelope-from gcooper@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [69.147.83.41]) by mx1.freebsd.org (Postfix) with ESMTP id AAF4313C441 for ; Sun, 15 Jul 2007 06:14:17 +0000 (UTC) (envelope-from gcooper@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id l6F6EHnJ025638 for ; Sun, 15 Jul 2007 06:14:17 GMT (envelope-from gcooper@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id l6F6EH5t025631 for perforce@freebsd.org; Sun, 15 Jul 2007 06:14:17 GMT (envelope-from gcooper@FreeBSD.org) Date: Sun, 15 Jul 2007 06:14:17 GMT Message-Id: <200707150614.l6F6EH5t025631@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to gcooper@FreeBSD.org using -f From: Garrett Cooper To: Perforce Change Reviews Cc: Subject: PERFORCE change 123521 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 15 Jul 2007 06:14:18 -0000 http://perforce.freebsd.org/chv.cgi?CH=123521 Change 123521 by gcooper@optimus-revised_pkgtools on 2007/07/15 06:13:22 - Buffered file reading algorithm from plist_add(..) moved to fileGetContentsByDescriptor - fileGetContents renamed to fileGetContentsByFilename(..), possibility for security-critical race condition when stat(2)'ing file reduced by using fopen(2) and fstat(2). - isUrl(..) logic slightly redone with constants instead of hardcoded strings and string lengths. Affected files ... .. //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/file.c#3 edit Differences ... ==== //depot/projects/soc2007/revised_fbsd_pkgtools/usr/src/usr.sbin/pkg_install/lib/file.c#3 (text+ko) ==== @@ -117,19 +117,33 @@ Boolean isURL(const char *fname) { + +#define URI_SUFFIX "://" +#define FILE_PREFIX "file" URI_SUFFIX +#define FTP_PREFIX "ftp" URI_SUFFIX +#define HTTP_PREFIX "http" URI_SUFFIX +#define HTTPS_PREFIX "https" URI_SUFFIX + /* * I'm sure there are other types of URL specifications that I could * also be looking for here, but for now I'll just be happy to get ftp * and http working. */ - if (!fname) - return FALSE; - while (isspace(*fname)) - ++fname; - if (!strncmp(fname, "ftp://", 6) || !strncmp(fname, "http://", 7) || - !strncmp(fname, "https://", 8) || !strncmp(fname, "file://", 7)) - return TRUE; + if (fname) { + + /** Get rid of leading whitespace **/ + while (isspace(*fname)) + ++fname; + + if (!strncmp(fname, FTP_PREFIX, strlen(FTP_PREFIX)) || !strncmp(fname, HTTP_PREFIX, strlen(HTTP_PREFIX)) || + !strncmp(fname, HTTPS_PREFIX, strlen(HTTPS_PREFIX)) || !strncmp(fname, FILE_PREFIX, strlen(FILE_PREFIX))) { + return TRUE; + } + + } + return FALSE; + } char * @@ -177,29 +191,43 @@ } char * -fileGetContents(const char *fname) +fileGetContentsByFilename(const char *fname) { char *contents; + + FILE *fd = fopen(fname, O_RDONLY); + + if (fd != NULL) { + cleanup(0); + errx(2, "%s: unable to open '%s' for reading", __func__, fname); + } + + contents = fileGetContentsByDescriptor(fd, fname); + + fclose(fd); + + return contents; + +} + +char * +fileGetContentsByDescriptor(FILE *fd, const char* fname) +{ + char *contents; struct stat sb; - int fd; - if (stat(fname, &sb) == FAIL) { + if (fstat(fileno(fd), &sb) == FAIL) { cleanup(0); errx(2, "%s: can't stat '%s'", __func__, fname); } contents = (char *)malloc(sb.st_size + 1); - fd = open(fname, O_RDONLY, 0); - if (fd == FAIL) { - cleanup(0); - errx(2, "%s: unable to open '%s' for reading", __func__, fname); - } - if (read(fd, contents, sb.st_size) != sb.st_size) { + + if ((int) fread(contents, sb.st_size, 1, fd) == FAIL) { cleanup(0); errx(2, "%s: short read on '%s' - did not get %lld bytes", __func__, fname, (long long)sb.st_size); } - close(fd); contents[sb.st_size] = '\0'; return contents; } @@ -287,7 +315,7 @@ time_diff.tv_sec = after.tv_sec - before.tv_sec; - printf( "(%s) Difference: %3.20lf secs\n", "copy_file", (double) ( time_diff.tv_sec + time_diff.tv_nsec/1e9 ) ); + fprintf(stderr, "(%s) Difference: %3.20lf secs\n", "copy_file", (double) ( time_diff.tv_sec + time_diff.tv_nsec/1e9 ) ); } @@ -315,7 +343,7 @@ time_diff.tv_sec = after.tv_sec - before.tv_sec; - printf( "(%s) Difference: %3.20lf secs\n", "move_file", (double) ( time_diff.tv_sec + time_diff.tv_nsec/1e9 ) ); + fprintf(stderr, "(%s) Difference: %3.20lf secs\n", "move_file", (double) ( time_diff.tv_sec + time_diff.tv_nsec/1e9 ) ); }