commit bc285942df401ea332667ce89f821648cc27d9cd Author: Thomas Habets Date: Fri Aug 30 16:48:39 2019 +0100 Check for size overflow in tar header fields. This prevents surprising outputs being created, e.g. this cpio tar output with more than one file: tar cf suffix.tar AUTHORS dd if=/dev/zero seek=16G bs=1 count=0 of=suffix.tar echo suffix.tar | cpio -H tar -o | tar tvf - -rw-r--r-- 1000/1000 0 2019-08-30 16:40 suffix.tar -rw-r--r-- thomas/thomas 161 2019-08-30 16:40 AUTHORS diff --git a/src/copyout.c b/src/copyout.c index 7532dac..22a3f9a 100644 --- a/src/copyout.c +++ b/src/copyout.c @@ -552,8 +552,7 @@ write_out_header (struct cpio_file_stat *file_hdr, int out_des) error (0, 0, _("%s: file name too long"), file_hdr->c_name); return 1; } - write_out_tar_header (file_hdr, out_des); /* FIXME: No error checking */ - return 0; + return write_out_tar_header (file_hdr, out_des); case arf_binary: return write_out_binary_header (makedev (file_hdr->c_rdev_maj, diff --git a/src/extern.h b/src/extern.h index 6fa2089..bb75b0b 100644 --- a/src/extern.h +++ b/src/extern.h @@ -144,7 +144,7 @@ int make_path (char *argpath, uid_t owner, gid_t group, const char *verbose_fmt_string); /* tar.c */ -void write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des); +int write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des); int null_block (long *block, int size); void read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des); int otoa (char *s, unsigned long *n); diff --git a/src/tar.c b/src/tar.c index 0a34845..98ff74d 100644 --- a/src/tar.c +++ b/src/tar.c @@ -91,8 +91,9 @@ stash_tar_filename (char *prefix, char *filename) sprintf (where, "%*lo ", digits - 2, value); except that sprintf fills in the trailing NUL and we don't. */ -static void -to_oct (register long value, register int digits, register char *where) +static int +to_oct_or_error (register long value, register int digits, register char *where, + const char *filename, const char *fieldname) { --digits; /* Leave the trailing NUL slot alone. */ @@ -103,10 +104,17 @@ to_oct (register long value, register int digits, register char *where) value >>= 3; } while (digits > 0 && value != 0); + if (value > 0) + { + error (0, 0, _("%s: field width not sufficient for storing %s"), + filename, fieldname); + return 1; + } /* Add leading zeroes, if necessary. */ while (digits > 0) where[--digits] = '0'; + return 0; } @@ -137,7 +145,7 @@ tar_checksum (struct tar_header *tar_hdr) /* Write out header FILE_HDR, including the file name, to file descriptor OUT_DES. */ -void +int write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des) { int name_len; @@ -166,11 +174,16 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des) /* Ustar standard (POSIX.1-1988) requires the mode to contain only 3 octal digits */ - to_oct (file_hdr->c_mode & MODE_ALL, 8, tar_hdr->mode); - to_oct (file_hdr->c_uid, 8, tar_hdr->uid); - to_oct (file_hdr->c_gid, 8, tar_hdr->gid); - to_oct (file_hdr->c_filesize, 12, tar_hdr->size); - to_oct (file_hdr->c_mtime, 12, tar_hdr->mtime); + if (to_oct_or_error (file_hdr->c_mode & MODE_ALL, 8, tar_hdr->mode, file_hdr->c_name, _("mode"))) + return 1; + if (to_oct_or_error (file_hdr->c_uid, 8, tar_hdr->uid, file_hdr->c_name, _("uid"))) + return 1; + if (to_oct_or_error (file_hdr->c_gid, 8, tar_hdr->gid, file_hdr->c_name, _("gid"))) + return 1; + if (to_oct_or_error (file_hdr->c_filesize, 12, tar_hdr->size, file_hdr->c_name, _("file size"))) + return 1; + if (to_oct_or_error (file_hdr->c_mtime, 12, tar_hdr->mtime, file_hdr->c_name, _("modification time"))) + return 1; switch (file_hdr->c_mode & CP_IFMT) { @@ -182,7 +195,8 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des) strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname, TARLINKNAMESIZE); tar_hdr->typeflag = LNKTYPE; - to_oct (0, 12, tar_hdr->size); + if (to_oct_or_error (0, 12, tar_hdr->size, file_hdr->c_name, _("file size"))) + return 1; } else tar_hdr->typeflag = REGTYPE; @@ -208,7 +222,8 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des) than TARLINKNAMESIZE. */ strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname, TARLINKNAMESIZE); - to_oct (0, 12, tar_hdr->size); + if (to_oct_or_error (0, 12, tar_hdr->size, file_hdr->c_name, _("file size"))) + return 1; break; #endif /* CP_IFLNK */ } @@ -227,13 +242,17 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des) if (name) strcpy (tar_hdr->gname, name); - to_oct (file_hdr->c_rdev_maj, 8, tar_hdr->devmajor); - to_oct (file_hdr->c_rdev_min, 8, tar_hdr->devminor); + if (to_oct_or_error (file_hdr->c_rdev_maj, 8, tar_hdr->devmajor, file_hdr->c_name, _("rdev major"))) + return 1; + if (to_oct_or_error (file_hdr->c_rdev_min, 8, tar_hdr->devminor, file_hdr->c_name, _("rdev minor"))) + return 1; } - to_oct (tar_checksum (tar_hdr), 8, tar_hdr->chksum); + if (to_oct_or_error (tar_checksum (tar_hdr), 8, tar_hdr->chksum, file_hdr->c_name, _("checksum"))) + return 1; tape_buffered_write ((char *) &tar_rec, out_des, TARRECORDSIZE); + return 0; } /* Return nonzero iff all the bytes in BLOCK are NUL.