Date: Thu, 01 Dec 2016 18:46:25 -0800 From: Cy Schubert <Cy.Schubert@komquats.com> To: Cy Schubert <Cy.Schubert@komquats.com> Cc: Martin Matuska <mm@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r309300 - in head: contrib/libarchive contrib/libarchive/libarchive contrib/libarchive/libarchive/test contrib/libarchive/tar/test lib/libarchive/tests Message-ID: <201612020246.uB22kPal051798@slippy.cwsent.com> In-Reply-To: Message from Cy Schubert <Cy.Schubert@komquats.com> of "Thu, 01 Dec 2016 18:25:18 -0800." <201612020225.uB22PIQl039020@slippy.cwsent.com>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <201612020225.uB22PIQl039020@slippy.cwsent.com>, Cy Schubert writes: > In message <201611292214.uATMEgQH079904@repo.freebsd.org>, Martin Matuska > write > s: > > Author: mm > > Date: Tue Nov 29 22:14:42 2016 > > New Revision: 309300 > > URL: https://svnweb.freebsd.org/changeset/base/309300 > > > > Log: > > MFV r309299: > > Sync libarchive with vendor. > > > > Important vendor bugfixes (relevant to FreeBSD): > > #821: tar -P cannot extract hardlinks through symlinks > > #825: Add sanity check of tar "uid, "gid" and "mtime" fields > > > > PR: 213255 > > Reported by: Tijl Coosemans <tilj@FreeBSD.org> > > MFC after: 1 week > > > > Added: > > head/contrib/libarchive/libarchive/test/test_compat_gtar_2.tar.uu > > - copied unchanged from r309299, vendor/libarchive/dist/libarchive/tes > t/ > > test_compat_gtar_2.tar.uu > > head/contrib/libarchive/libarchive/test/test_compat_star_acl_posix1e.c > > - copied unchanged from r309299, vendor/libarchive/dist/libarchive/tes > t/ > > test_compat_star_acl_posix1e.c > > head/contrib/libarchive/libarchive/test/test_compat_star_acl_posix1e.tar. > uu > > - copied unchanged from r309299, vendor/libarchive/dist/libarchive/tes > t/ > > test_compat_star_acl_posix1e.tar.uu > > head/contrib/libarchive/libarchive/test/test_read_format_raw.bufr.uu > > - copied unchanged from r309299, vendor/libarchive/dist/libarchive/tes > t/ > > test_read_format_raw.bufr.uu > > Modified: > > head/contrib/libarchive/NEWS > > head/contrib/libarchive/libarchive/archive_acl.c > > head/contrib/libarchive/libarchive/archive_entry.c > > head/contrib/libarchive/libarchive/archive_entry.h > > head/contrib/libarchive/libarchive/archive_entry_acl.3 > > head/contrib/libarchive/libarchive/archive_read_disk_entry_from_file.c > > head/contrib/libarchive/libarchive/archive_read_support_filter_xz.c > > head/contrib/libarchive/libarchive/archive_read_support_format_tar.c > > head/contrib/libarchive/libarchive/archive_read_support_format_xar.c > > head/contrib/libarchive/libarchive/archive_write_disk_posix.c > > head/contrib/libarchive/libarchive/test/test_compat_gtar.c > > head/contrib/libarchive/libarchive/test/test_read_format_raw.c > > head/contrib/libarchive/libarchive/test/test_sparse_basic.c > > head/contrib/libarchive/tar/test/test_symlink_dir.c > > head/lib/libarchive/tests/Makefile > > Directory Properties: > > head/contrib/libarchive/ (props changed) > > > [...] > > Modified: head/contrib/libarchive/libarchive/archive_read_support_format_ta > r. > > c > > =========================================================================== > == > > = > > --- head/contrib/libarchive/libarchive/archive_read_support_format_tar.c > > Tue Nov 29 21:53:16 2016 (r309299) > > +++ head/contrib/libarchive/libarchive/archive_read_support_format_tar.c > > Tue Nov 29 22:14:42 2016 (r309300) > > @@ -294,6 +294,46 @@ archive_read_format_tar_cleanup(struct a > > return (ARCHIVE_OK); > > } > > > > +static int > > +validate_number_field(const char* p_field, size_t i_size) > > +{ > > + unsigned char marker = (unsigned char)p_field[0]; > > + /* octal? */ > > + if ((marker >= '0' && marker <= '7') || marker == ' ') { > > + size_t i = 0; > > + int octal_found = 0; > > + for (i = 0; i < i_size; ++i) { > > + switch (p_field[i]) > > + { > > + case ' ': /* skip any leading spaces and trailing space > > */ > > + if (octal_found == 0 || i == i_size - 1) { > > + continue; > > + } > > + break; > > + case '\0': /* null is allowed only at the end */ > > + if (i != i_size - 1) { > > + return 0; > > + } > > + break; > > + /* rest must be octal digits */ > > + case '0': case '1': case '2': case '3': > > + case '4': case '5': case '6': case '7': > > + ++octal_found; > > + break; > > + } > > + } > > + return octal_found > 0; > > + } > > + /* base 256 (i.e. binary number) */ > > + else if (marker == 128 || marker == 255 || marker == 0) { > > + /* nothing to check */ > > + return 1; > > + } > > + /* not a number field */ > > + else { > > + return 0; > > + } > > +} > > > > static int > > archive_read_format_tar_bid(struct archive_read *a, int best_bid) > > @@ -346,23 +386,23 @@ archive_read_format_tar_bid(struct archi > > return (0); > > bid += 2; /* 6 bits of variation in an 8-bit field leaves 2 bits. */ > > > > - /* Sanity check: Look at first byte of mode field. */ > > - switch (255 & (unsigned)header->mode[0]) { > > - case 0: case 255: > > - /* Base-256 value: No further verification possible! */ > > - break; > > - case ' ': /* Not recommended, but not illegal, either. */ > > - break; > > - case '0': case '1': case '2': case '3': > > - case '4': case '5': case '6': case '7': > > - /* Octal Value. */ > > - /* TODO: Check format of remainder of this field. */ > > - break; > > - default: > > - /* Not a valid mode; bail out here. */ > > - return (0); > > + /* > > + * Check format of mode/uid/gid/mtime/size/rdevmajor/rdevminor fields. > > + * These are usually octal numbers but GNU tar encodes "big" values as > > + * base256 and leading zeroes are sometimes replaced by spaces. > > + * Even the null terminator is sometimes omitted. Anyway, must be check > > ed > > + * to avoid false positives. > > + */ > > + if (bid > 0 && > > + (validate_number_field(header->mode, sizeof(header->mode)) == 0 > > || > > + validate_number_field(header->uid, sizeof(header->uid)) == 0 | > > | > > + validate_number_field(header->gid, sizeof(header->gid)) == 0 | > > | > > + validate_number_field(header->mtime, sizeof(header->mtime)) == > > 0 || > > + validate_number_field(header->size, sizeof(header->size)) == 0 > > || > > + validate_number_field(header->rdevmajor, sizeof(header->rdevma > > jor)) == 0 || > > + validate_number_field(header->rdevminor, sizeof(header->rdevmi > > nor)) == 0)) { > > + bid = 0; > > } > > - /* TODO: Sanity test uid/gid/size/mtime/rdevmajor/rdevminor fields. */ > > > > return (bid); > > } > > > > Hi, > > This broke extract of older tarballs and compatibility with Perl > Archive::Tar. You can test this by trying to extract ports/net/tcpview. > > The following patch fixes it: > > https://github.com/libarchive/libarchive/commit/2d2b3e928605f795515b03f060fd6 > 38c265b0778#diff-845deb76aa98ded5cb2860ded3e19dfb > > The relevant part of the patch follows: > > diff --git a/libarchive/archive_read_support_format_tar.c b/libarchive/archiv > e_read_support_format_tar.c > index 0ee511e..b977cb7 100644 > --- libarchive/archive_read_support_format_tar.c > +++ libarchive/archive_read_support_format_tar.c > @@ -294,8 +294,14 @@ archive_read_format_tar_cleanup(struct archive_read *a) > return (ARCHIVE_OK); > } > > +/* > + * Validate number field > + * > + * Flags: > + * 1 - allow double \0 at field end > + */ > static int > -validate_number_field(const char* p_field, size_t i_size) > +validate_number_field(const char* p_field, size_t i_size, int flags) > { > unsigned char marker = (unsigned char)p_field[0]; > /* octal? */ > @@ -305,14 +311,24 @@ validate_number_field(const char* p_field, size_t i_siz > e) > for (i = 0; i < i_size; ++i) { > switch (p_field[i]) > { > - case ' ': /* skip any leading spaces and trailing space > */ > + case ' ': > + /* skip any leading spaces and trailing space * > / > if (octal_found == 0 || i == i_size - 1) { > continue; > } > break; > - case '\0': /* null is allowed only at the end */ > + case '\0': > + /* > + * null should be allowed only at the end > + * > + * Perl Archive::Tar terminates some fields > + * with two nulls. We must allow this to stay > + * compatible. > + */ > if (i != i_size - 1) { > - return 0; > + if (((flags & 1) == 0) > + || i != i_size - 2) > + return 0; > } > break; > /* rest must be octal digits */ > @@ -390,18 +406,25 @@ archive_read_format_tar_bid(struct archive_read *a, int > best_bid) > * Check format of mode/uid/gid/mtime/size/rdevmajor/rdevminor fields. > * These are usually octal numbers but GNU tar encodes "big" values as > * base256 and leading zeroes are sometimes replaced by spaces. > - * Even the null terminator is sometimes omitted. Anyway, must be check > ed > - * to avoid false positives. > + * Even the null terminator is sometimes omitted. Anyway, must be > + * checked to avoid false positives. > + * > + * Perl Archive::Tar does not follow the spec and terminates mode, uid, > + * gid, rdevmajor and rdevminor with a double \0. For compatibility > + * reasons we allow this deviation. > */ > - if (bid > 0 && > - (validate_number_field(header->mode, sizeof(header->mode)) == 0 > || > - validate_number_field(header->uid, sizeof(header->uid)) == 0 | > | > - validate_number_field(header->gid, sizeof(header->gid)) == 0 | > | > - validate_number_field(header->mtime, sizeof(header->mtime)) == > 0 || > - validate_number_field(header->size, sizeof(header->size)) == 0 > || > - validate_number_field(header->rdevmajor, sizeof(header->rdevma > jor)) == 0 || > - validate_number_field(header->rdevminor, sizeof(header->rdevmi > nor)) == 0)) { > - bid = 0; > + if (bid > 0 && ( > + validate_number_field(header->mode, sizeof(header->mode), 1) == 0 > + || validate_number_field(header->uid, sizeof(header->uid), 1) == 0 > + || validate_number_field(header->gid, sizeof(header->gid), 1) == 0 > + || validate_number_field(header->mtime, sizeof(header->mtime), > + 0) == 0 > + || validate_number_field(header->size, sizeof(header->size), 0) == > 0 > + || validate_number_field(header->rdevmajor, > + sizeof(header->rdevmajor), 1) == 0 > + || validate_number_field(header->rdevminor, > + sizeof(header->rdevminor), 1) == 0)) { > + bid = 0; > } > > return (bid); > > Updating to git 2d2b3e928605f795515b03f060fd638c265b0778 or later probably > makes the most sense. Sorry for the spam. I'm a day out of date. :( -- Cheers, Cy Schubert <Cy.Schubert@cschubert.com> FreeBSD UNIX: <cy@FreeBSD.org> Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201612020246.uB22kPal051798>