From owner-svn-src-head@freebsd.org Fri Dec 2 02:46:36 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4DDFDC62C92; Fri, 2 Dec 2016 02:46:36 +0000 (UTC) (envelope-from cy.schubert@komquats.com) Received: from smtp-out-so.shaw.ca (smtp-out-so.shaw.ca [64.59.136.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id EC0431B81; Fri, 2 Dec 2016 02:46:35 +0000 (UTC) (envelope-from cy.schubert@komquats.com) Received: from spqr.komquats.com ([96.50.22.10]) by shaw.ca with SMTP id CdrncbTunIwqSCdrocdIXH; Thu, 01 Dec 2016 19:46:29 -0700 X-Authority-Analysis: v=2.2 cv=cNuQihWN c=1 sm=1 tr=0 a=jvE2nwUzI0ECrNeyr98KWA==:117 a=jvE2nwUzI0ECrNeyr98KWA==:17 a=kj9zAlcOel0A:10 a=n5n_aSjo0skA:10 a=VxmjJ2MpAAAA:8 a=6I5d2MoRAAAA:8 a=NEAV23lmAAAA:8 a=YxBL1-UpAAAA:8 a=dPdGhu47XFbj6EuJ0X4A:9 a=N4WJNfBsDPN8pCut:21 a=5hvrOP4l-_IyPNkK:21 a=pynXk66uh0YmACDl:21 a=CjuIK1q_8ugA:10 a=7gXAzLPJhVmCkEl4_tsf:22 a=IjZwj45LgO3ly-622nXo:22 a=Bn2pgwyD2vrAyMmN8A2t:22 a=Ia-lj3WSrqcvXOmTRaiG:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTPS id 5F5AFC7B; Thu, 1 Dec 2016 17:32:44 -0800 (PST) Received: from slippy (localhost [127.0.0.1]) by slippy.cwsent.com (8.15.2/8.15.2) with ESMTP id uB22kPal051798; Thu, 1 Dec 2016 18:46:25 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <201612020246.uB22kPal051798@slippy.cwsent.com> X-Mailer: exmh version 2.8.0 04/21/2012 with nmh-1.6 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Cy Schubert cc: Martin Matuska , 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 In-Reply-To: Message from Cy Schubert of "Thu, 01 Dec 2016 18:25:18 -0800." <201612020225.uB22PIQl039020@slippy.cwsent.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Thu, 01 Dec 2016 18:46:25 -0800 X-CMAE-Envelope: MS4wfEizk5up1OXAZWECNpCHdLpi/W4xDMK55jro6rSToR8XNzsF41D1WjQai/V9jlQKJdcMwsWH5AOVSLCHQDkBiYWw/xn5bvqwbKO8WyXgJdvrlBykQqf1 x8/U+vCKnZTZ+rcL6AhGmrHk1MWzecSAIbgQ4XOi9z14NwjNgfwKsGLgUpOEY+1YMwYBJNVfBZVtH76ED61uQfIRDHgCjWSWedhuLV0aWt/E3omxYvc0UYfi OUZNF/KWFn+R6A+3ytoJtdV0DZNn28ksF7asDgVWrhN16iQMZNKOPGvqmeble50S X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Dec 2016 02:46:36 -0000 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 > > 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 FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few.