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/test/
> 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/test/
> 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/test/
> 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/test/
> 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_tar.
> 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/2d2b3e928605f795515b03f060fd638c265b0778#diff-845deb76aa98ded5cb2860ded3e19dfb

The relevant part of the patch follows:

diff --git a/libarchive/archive_read_support_format_tar.c 
b/libarchive/archive_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_size)
                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 
checked
-        * 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->rdevmajor)) == 0 ||
-                validate_number_field(header->rdevminor, 
sizeof(header->rdevminor)) == 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.

 

-- 
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