In message <201612020225.ub22piql039...@slippy.cwsent.com>, Cy Schubert 
writes:
> In message <201611292214.uatmegqh079...@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 <t...@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.schub...@cschubert.com>
FreeBSD UNIX:  <c...@freebsd.org>   Web:  http://www.FreeBSD.org

        The need of the many outweighs the greed of the few.


_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to