From 140c58ca2739ba49a909c87a3c29ebc86724292b Mon Sep 17 00:00:00 2001 From: cperciva Date: Wed, 11 Jan 2006 08:02:16 +0000 Subject: Correct insecure temporary file usage in texindex. [06:01] Correct insecure temporary file usage in ee. [06:02] Correct a race condition when setting file permissions, sanitize file names by default, and fix a buffer overflow when handling files larger than 4GB in cpio. [06:03] Fix an error in the handling of IP fragments in ipfw which can cause a kernel panic. [06:04] Security: FreeBSD-SA-06:01.texindex Security: FreeBSD-SA-06:02.ee Security: FreeBSD-SA-06:03.cpio Security: FreeBSD-SA-06:04.ipfw --- contrib/cpio/doc/cpio.1 | 2 +- contrib/cpio/doc/cpio.texi | 8 +-- contrib/cpio/src/copyin.c | 111 ++++++++++++++++++++++++++++++---------- contrib/cpio/src/copyout.c | 22 +++++++- contrib/cpio/src/copypass.c | 16 ++++-- contrib/cpio/src/extern.h | 2 +- contrib/cpio/src/global.c | 2 +- contrib/cpio/src/main.c | 13 +++-- contrib/texinfo/util/texindex.c | 65 ++++++++++++++++++++++- 9 files changed, 194 insertions(+), 47 deletions(-) (limited to 'contrib') diff --git a/contrib/cpio/doc/cpio.1 b/contrib/cpio/doc/cpio.1 index dc3eb80..a59094a 100644 --- a/contrib/cpio/doc/cpio.1 +++ b/contrib/cpio/doc/cpio.1 @@ -20,7 +20,7 @@ cpio \- copy files to and from archives [\-\-unconditional] [\-\-verbose] [\-\-block-size=blocks] [\-\-swap-halfwords] [\-\-io-size=bytes] [\-\-pattern-file=file] [\-\-format=format] [\-\-owner=[user][:.][group]] [\-\-no-preserve-owner] [\-\-message=message] -[\-\-force\-local] [\-\-no\-absolute\-filenames] [\-\-sparse] +[\-\-force\-local] [\-\-absolute\-filenames] [\-\-sparse] [\-\-only\-verify\-crc] [\-\-quiet] [\-\-rsh-command=command] [\-\-help] [\-\-version] [pattern...] [< archive] diff --git a/contrib/cpio/doc/cpio.texi b/contrib/cpio/doc/cpio.texi index ae534f4..b7b8610 100644 --- a/contrib/cpio/doc/cpio.texi +++ b/contrib/cpio/doc/cpio.texi @@ -288,7 +288,7 @@ cpio @{-i|--extract@} [-bcdfmnrtsuvBSV] [-C bytes] [-E file] [--swap-halfwords] [--io-size=bytes] [--pattern-file=file] [--format=format] [--owner=[user][:.][group]] [--no-preserve-owner] [--message=message] [--help] [--version] -[-no-absolute-filenames] [--sparse] [-only-verify-crc] [-quiet] +[--absolute-filenames] [--sparse] [-only-verify-crc] [-quiet] [--rsh-command=command] [pattern...] [< archive] @end example @@ -451,9 +451,9 @@ current volume number (starting at 1). Show numeric UID and GID instead of translating them into names when using the @samp{--verbose option}. -@item --no-absolute-filenames -Create all files relative to the current directory in copy-in mode, even -if they have an absolute file name in the archive. +@item --absolute-filenames +Do not strip leading file name components that contain ".." and +leading slashes from file names in copy-in mode @item --no-preserve-owner Do not change the ownership of the files; leave them owned by the user diff --git a/contrib/cpio/src/copyin.c b/contrib/cpio/src/copyin.c index a3e186a..3e0a737 100644 --- a/contrib/cpio/src/copyin.c +++ b/contrib/cpio/src/copyin.c @@ -27,6 +27,7 @@ #include "dstring.h" #include "extern.h" #include "defer.h" +#include "dirname.h" #include #ifndef FNM_PATHNAME #include @@ -392,19 +393,26 @@ create_final_defers () continue; } - if (close (out_file_des) < 0) - error (0, errno, "%s", d->header.c_name); - + /* + * Avoid race condition. + * Set chown and chmod before closing the file desc. + * pvrabec@redhat.com + */ + /* File is now copied; set attributes. */ if (!no_chown_flag) - if ((chown (d->header.c_name, + if ((fchown (out_file_des, set_owner_flag ? set_owner : d->header.c_uid, set_group_flag ? set_group : d->header.c_gid) < 0) && errno != EPERM) error (0, errno, "%s", d->header.c_name); /* chown may have turned off some permissions we wanted. */ - if (chmod (d->header.c_name, (int) d->header.c_mode) < 0) + if (fchmod (out_file_des, (int) d->header.c_mode) < 0) error (0, errno, "%s", d->header.c_name); + + if (close (out_file_des) < 0) + error (0, errno, "%s", d->header.c_name); + if (retain_time_flag) { times.actime = times.modtime = d->header.c_mtime; @@ -560,6 +568,25 @@ copyin_regular_file (struct new_cpio_header* file_hdr, int in_file_des) write (out_file_des, "", 1); delayed_seek_count = 0; } + + /* + * Avoid race condition. + * Set chown and chmod before closing the file desc. + * pvrabec@redhat.com + */ + + /* File is now copied; set attributes. */ + if (!no_chown_flag) + if ((fchown (out_file_des, + set_owner_flag ? set_owner : file_hdr->c_uid, + set_group_flag ? set_group : file_hdr->c_gid) < 0) + && errno != EPERM) + error (0, errno, "%s", file_hdr->c_name); + + /* chown may have turned off some permissions we wanted. */ + if (fchmod (out_file_des, (int) file_hdr->c_mode) < 0) + error (0, errno, "%s", file_hdr->c_name); + if (close (out_file_des) < 0) error (0, errno, "%s", file_hdr->c_name); @@ -570,18 +597,6 @@ copyin_regular_file (struct new_cpio_header* file_hdr, int in_file_des) file_hdr->c_name, crc, file_hdr->c_chksum); } - /* File is now copied; set attributes. */ - if (!no_chown_flag) - if ((chown (file_hdr->c_name, - set_owner_flag ? set_owner : file_hdr->c_uid, - set_group_flag ? set_group : file_hdr->c_gid) < 0) - && errno != EPERM) - error (0, errno, "%s", file_hdr->c_name); - - /* chown may have turned off some permissions we wanted. */ - if (chmod (file_hdr->c_name, (int) file_hdr->c_mode) < 0) - error (0, errno, "%s", file_hdr->c_name); - if (retain_time_flag) { struct utimbuf times; /* For setting file times. */ @@ -592,7 +607,7 @@ copyin_regular_file (struct new_cpio_header* file_hdr, int in_file_des) if (utime (file_hdr->c_name, ×) < 0) error (0, errno, "%s", file_hdr->c_name); } - + tape_skip_padding (in_file_des, file_hdr->c_filesize); if (file_hdr->c_nlink > 1 && (archive_format == arf_newascii || archive_format == arf_crcascii) ) @@ -1350,6 +1365,53 @@ swab_array (char *ptr, int count) } } +/* Return a safer suffix of FILE_NAME, or "." if it has no safer + suffix. Check for fully specified file names and other atrocities. */ + +static const char * +safer_name_suffix (char const *file_name) +{ + char const *p; + + /* Skip file system prefixes, leading file name components that contain + "..", and leading slashes. */ + + size_t prefix_len = FILE_SYSTEM_PREFIX_LEN (file_name); + + for (p = file_name + prefix_len; *p;) + { + if (p[0] == '.' && p[1] == '.' && (ISSLASH (p[2]) || !p[2])) + prefix_len = p + 2 - file_name; + + do + { + char c = *p++; + if (ISSLASH (c)) + break; + } + while (*p); + } + + for (p = file_name + prefix_len; ISSLASH (*p); p++) + continue; + prefix_len = p - file_name; + + if (prefix_len) + { + char *prefix = alloca (prefix_len + 1); + memcpy (prefix, file_name, prefix_len); + prefix[prefix_len] = '\0'; + + + error (0, 0, _("Removing leading `%s' from member names"), prefix); + } + + if (!*p) + p = "."; + + return p; +} + /* Read the collection from standard input and create files in the file system. */ @@ -1460,18 +1522,11 @@ process_copy_in () /* Do we have to ignore absolute paths, and if so, does the filename have an absolute path? */ - if (no_abs_paths_flag && file_hdr.c_name && file_hdr.c_name [0] == '/') + if (!abs_paths_flag && file_hdr.c_name && file_hdr.c_name [0]) { - char *p; + const char *p = safer_name_suffix (file_hdr.c_name); - p = file_hdr.c_name; - while (*p == '/') - ++p; - if (*p == '\0') - { - strcpy (file_hdr.c_name, "."); - } - else + if (p != file_hdr.c_name) { /* Debian hack: file_hrd.c_name is sometimes set to point to static memory by code in tar.c. This diff --git a/contrib/cpio/src/copyout.c b/contrib/cpio/src/copyout.c index 75d076b..63cb0e2 100644 --- a/contrib/cpio/src/copyout.c +++ b/contrib/cpio/src/copyout.c @@ -303,12 +303,13 @@ write_out_header (struct new_cpio_header *file_hdr, int out_des) { char ascii_header[112]; char *magic_string; + int ret; if (archive_format == arf_crcascii) magic_string = "070702"; else magic_string = "070701"; - sprintf (ascii_header, + ret = snprintf (ascii_header, sizeof(ascii_header), "%6s%08lx%08lx%08lx%08lx%08lx%08lx%08lx%08lx%08lx%08lx%08lx%08lx%08lx", magic_string, file_hdr->c_ino, file_hdr->c_mode, file_hdr->c_uid, @@ -316,6 +317,10 @@ write_out_header (struct new_cpio_header *file_hdr, int out_des) file_hdr->c_filesize, file_hdr->c_dev_maj, file_hdr->c_dev_min, file_hdr->c_rdev_maj, file_hdr->c_rdev_min, file_hdr->c_namesize, file_hdr->c_chksum); + if (ret >= sizeof(ascii_header)) { + fprintf(stderr, "Internal overflow, aborting\n"); + exit (1); + } tape_buffered_write (ascii_header, out_des, 110L); /* Write file name to output. */ @@ -325,6 +330,7 @@ write_out_header (struct new_cpio_header *file_hdr, int out_des) else if (archive_format == arf_oldascii || archive_format == arf_hpoldascii) { char ascii_header[78]; + int ret; dev_t dev; dev_t rdev; @@ -365,7 +371,7 @@ write_out_header (struct new_cpio_header *file_hdr, int out_des) /* Debian hack: The type of dev_t has changed in glibc. Fixed output to ensure that a long int is passed to sprintf. This has been reported to "bug-gnu-utils@prep.ai.mit.edu". (1998/5/26) -BEM */ - sprintf (ascii_header, + snprintf (ascii_header, sizeof(ascii_header), "%06ho%06lo%06lo%06lo%06lo%06lo%06lo%06lo%011lo%06lo%011lo", file_hdr->c_magic & 0xFFFF, (long) dev & 0xFFFF, file_hdr->c_ino & 0xFFFF, file_hdr->c_mode & 0xFFFF, @@ -373,6 +379,10 @@ write_out_header (struct new_cpio_header *file_hdr, int out_des) file_hdr->c_nlink & 0xFFFF, (long) rdev & 0xFFFF, file_hdr->c_mtime, file_hdr->c_namesize & 0xFFFF, file_hdr->c_filesize); + if (ret >= sizeof(ascii_header)) { + fprintf(stderr, "Internal overflow, aborting\n"); + exit (1); + } tape_buffered_write (ascii_header, out_des, 76L); /* Write file name to output. */ @@ -508,6 +518,14 @@ process_copy_out () file_hdr.c_dev_maj = major (file_stat.st_dev); file_hdr.c_dev_min = minor (file_stat.st_dev); file_hdr.c_ino = file_stat.st_ino; + + /* Skip files larger than 4GB which will cause problems on + 64bit platforms (and just not work on 32bit). */ + if (file_stat.st_size > 0xffffffff) { + error (0, 0, "%s: skipping >4GB file", input_name.ds_string); + continue; + } + /* For POSIX systems that don't define the S_IF macros, we can't assume that S_ISfoo means the standard Unix S_IFfoo bit(s) are set. So do it manually, with a diff --git a/contrib/cpio/src/copypass.c b/contrib/cpio/src/copypass.c index 6cef1e8..640f24e 100644 --- a/contrib/cpio/src/copypass.c +++ b/contrib/cpio/src/copypass.c @@ -183,19 +183,25 @@ process_copy_pass () } if (close (in_file_des) < 0) error (0, errno, "%s", input_name.ds_string); - if (close (out_file_des) < 0) - error (0, errno, "%s", output_name.ds_string); - + /* + * Avoid race condition. + * Set chown and chmod before closing the file desc. + * pvrabec@redhat.com + */ /* Set the attributes of the new file. */ if (!no_chown_flag) - if ((chown (output_name.ds_string, + if ((fchown (out_file_des, set_owner_flag ? set_owner : in_file_stat.st_uid, set_group_flag ? set_group : in_file_stat.st_gid) < 0) && errno != EPERM) error (0, errno, "%s", output_name.ds_string); /* chown may have turned off some permissions we wanted. */ - if (chmod (output_name.ds_string, in_file_stat.st_mode) < 0) + if (fchmod (out_file_des, in_file_stat.st_mode) < 0) + error (0, errno, "%s", output_name.ds_string); + + if (close (out_file_des) < 0) error (0, errno, "%s", output_name.ds_string); + if (reset_time_flag) { times.actime = in_file_stat.st_atime; diff --git a/contrib/cpio/src/extern.h b/contrib/cpio/src/extern.h index e6e3d6e..b1ed8ef5 100644 --- a/contrib/cpio/src/extern.h +++ b/contrib/cpio/src/extern.h @@ -46,7 +46,7 @@ extern int no_chown_flag; extern int sparse_flag; extern int quiet_flag; extern int only_verify_crc_flag; -extern int no_abs_paths_flag; +extern int abs_paths_flag; extern unsigned int warn_option; /* Values for warn_option */ diff --git a/contrib/cpio/src/global.c b/contrib/cpio/src/global.c index 97b6bea..d7484ad 100644 --- a/contrib/cpio/src/global.c +++ b/contrib/cpio/src/global.c @@ -100,7 +100,7 @@ int quiet_flag = false; int only_verify_crc_flag = false; /* If true, don't use any absolute paths, prefix them by `./'. */ -int no_abs_paths_flag = false; +int abs_paths_flag = false; #ifdef DEBUG_CPIO /* If true, print debugging information. */ diff --git a/contrib/cpio/src/main.c b/contrib/cpio/src/main.c index de59074..f689520 100644 --- a/contrib/cpio/src/main.c +++ b/contrib/cpio/src/main.c @@ -43,6 +43,7 @@ enum cpio_options { NO_ABSOLUTE_FILENAMES_OPTION=256, + ABSOLUTE_FILENAMES_OPTION, NO_PRESERVE_OWNER_OPTION, ONLY_VERIFY_CRC_OPTION, RENAME_BATCH_FILE_OPTION, @@ -136,6 +137,8 @@ static struct argp_option options[] = { N_("In copy-in mode, read additional patterns specifying filenames to extract or list from FILE"), 210}, {"no-absolute-filenames", NO_ABSOLUTE_FILENAMES_OPTION, 0, 0, N_("Create all files relative to the current directory"), 210}, + {"absolute-filenames", ABSOLUTE_FILENAMES_OPTION, 0, 0, + N_("do not strip leading file name components that contain \"..\" and leading slashes from file names"), 210}, {"only-verify-crc", ONLY_VERIFY_CRC_OPTION, 0, 0, N_("When reading a CRC format archive in copy-in mode, only verify the CRC's of each file in the archive, don't actually extract the files"), 210}, {"rename", 'r', 0, 0, @@ -394,7 +397,11 @@ crc newc odc bin ustar tar (all-caps also recognized)"), arg); break; case NO_ABSOLUTE_FILENAMES_OPTION: /* --no-absolute-filenames */ - no_abs_paths_flag = true; + abs_paths_flag = false; + break; + + case ABSOLUTE_FILENAMES_OPTION: /* --absolute-filenames */ + abs_paths_flag = true; break; case NO_PRESERVE_OWNER_OPTION: /* --no-preserve-owner */ @@ -633,7 +640,7 @@ process_args (int argc, char *argv[]) _("--append is used but no archive file name is given (use -F or -O options"))); CHECK_USAGE(rename_batch_file, "--rename-batch-file", "--create"); - CHECK_USAGE(no_abs_paths_flag, "--no-absolute-pathnames", "--create"); + CHECK_USAGE(abs_paths_flag, "--absolute-pathnames", "--create"); CHECK_USAGE(input_archive_name, "-I", "--create"); if (archive_name && output_archive_name) USAGE_ERROR ((0, 0, _("Both -O and -F are used in copy-out mode"))); @@ -660,7 +667,7 @@ process_args (int argc, char *argv[]) CHECK_USAGE(rename_flag, "--rename", "--pass-through"); CHECK_USAGE(append_flag, "--append", "--pass-through"); CHECK_USAGE(rename_batch_file, "--rename-batch-file", "--pass-through"); - CHECK_USAGE(no_abs_paths_flag, "--no-absolute-pathnames", + CHECK_USAGE(abs_paths_flag, "--absolute-pathnames", "--pass-through"); CHECK_USAGE(to_stdout_option, "--to-stdout", "--pass-through"); diff --git a/contrib/texinfo/util/texindex.c b/contrib/texinfo/util/texindex.c index 569f877..0718e72 100644 --- a/contrib/texinfo/util/texindex.c +++ b/contrib/texinfo/util/texindex.c @@ -384,17 +384,33 @@ For more information about these matters, see the files named COPYING.\n")); usage (1); } +static char **tv; +static int tv_alloc; +static int tv_used; + +static int +findtempname (char *tempname) +{ + int i; + + for (i = 0; i < tv_used; i++) + if (strcmp (tv[i], tempname) == 0) + return (1); + return (0); +} + /* Return a name for temporary file COUNT. */ static char * maketempname (int count) { static char *tempbase = NULL; + char *tempname; char tempsuffix[10]; + int fd; if (!tempbase) { - int fd; tempbase = concat (tempdir, "txidxXXXXXX"); fd = mkstemp (tempbase); @@ -403,7 +419,52 @@ maketempname (int count) } sprintf (tempsuffix, ".%d", count); - return concat (tempbase, tempsuffix); + tempname = concat (tempbase, tempsuffix); + /* + * The open logic becomes a bit convoluted. If open(2) fails due to EEXIST, + * it's likely because somebody attempted to race us, or because we have + * already created this file. + */ + fd = open (tempname, O_CREAT|O_EXCL|O_WRONLY, 0600); + if (fd == -1) + { + /* + * If errno is not EEXIST, then open failed for some other reason, so + * we should terminate. If errno == EEXIST AND we didn't create this + * file, terminate. Otherwise, it's safe to say that errno == EEXIST + * because we already created it, in this event, we can just return. + */ + if (errno != EEXIST || + (errno == EEXIST && findtempname (tempname) == 0)) + pfatal_with_name (tempname); + return (tempname); + } + else if (fd > 0) + { + close (fd); + } + if (tv == NULL) + { + tv_alloc = 16; + tv = calloc (tv_alloc, sizeof (char *)); + if (tv == NULL) + { + fprintf (stderr, "calloc failed\n"); + exit (1); + } + } + else if (tv_used == tv_alloc) + { + tv_alloc += 4; + tv = realloc (tv, tv_alloc * sizeof (char *)); + if (tv == NULL) + { + fprintf (stderr, "realloc failed"); + exit (1); + } + } + tv[tv_used++] = strdup (tempname); + return tempname; } -- cgit v1.1