Polish for GNU tar format reading/writing (#2455)

A few small tweaks to improve reading/writing of the legacy GNU tar
format.

* Be more tolerant of redundant 'K' and 'L' headers
* Fill in missing error messages for redundant headers
* New test for reading archive with redundant 'L' headers
* Earlier identification of GNU tar format in some cases

These changes were inspired by Issue #2434. Although that was determined
to not technically be a bug in libarchive, it's relatively easy for
libarchive to tolerate duplicate 'K' and 'L' headers and we should be
issuing appropriate error messages in any case.

diff --git a/Makefile.am b/Makefile.am
index c6d072cf318e46791557b4d7c08300170af31166..5d7abc13bcb39cf4f161efba47cfb7394aa75c67 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -491,6 +491,7 @@ libarchive_test_SOURCES= \
        libarchive/test/test_read_format_gtar_filename.c \
        libarchive/test/test_read_format_gtar_gz.c \
        libarchive/test/test_read_format_gtar_lzma.c \
+       libarchive/test/test_read_format_gtar_redundant_L.c \
        libarchive/test/test_read_format_gtar_sparse.c \
        libarchive/test/test_read_format_gtar_sparse_length.c \
        libarchive/test/test_read_format_gtar_sparse_skip_entry.c \
@@ -843,6 +844,7 @@ libarchive_test_EXTRA_DIST=\
        libarchive/test/test_read_format_gtar_filename_cp866.tar.Z.uu \
        libarchive/test/test_read_format_gtar_filename_eucjp.tar.Z.uu \
        libarchive/test/test_read_format_gtar_filename_koi8r.tar.Z.uu \
+       libarchive/test/test_read_format_gtar_redundant_L.tar.Z.uu \
        libarchive/test/test_read_format_gtar_sparse_1_13.tar.uu \
        libarchive/test/test_read_format_gtar_sparse_1_17.tar.uu \
        libarchive/test/test_read_format_gtar_sparse_1_17_posix00.tar.uu \

diff --git a/libarchive/archive_read_support_format_tar.c b/libarchive/archive_read_support_format_tar.c
index 4ef21d443eac824790b5ba4e9512e79f4a5c929c..7d212906d7ac7f235d80d984f804ae674fdbdde4 100644 (file)
--- a/libarchive/archive_read_support_format_tar.c
+++ b/libarchive/archive_read_support_format_tar.c
@@ -816,6 +816,8 @@ tar_read_header(struct archive_read *a, struct tar *tar,
                switch(header->typeflag[0]) {
                case 'A': /* Solaris tar ACL */
                        if (seen_headers & seen_A_header) {
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Redundant 'A' header");
                                return (ARCHIVE_FATAL);
                        }
                        seen_headers |= seen_A_header;
@@ -825,6 +827,8 @@ tar_read_header(struct archive_read *a, struct tar *tar,
                        break;
                case 'g': /* POSIX-standard 'g' header. */
                        if (seen_headers & seen_g_header) {
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Redundant 'g' header");
                                return (ARCHIVE_FATAL);
                        }
                        seen_headers |= seen_g_header;
@@ -834,27 +838,41 @@ tar_read_header(struct archive_read *a, struct tar *tar,
                        break;
                case 'K': /* Long link name (GNU tar, others) */
                        if (seen_headers & seen_K_header) {
-                               return (ARCHIVE_FATAL);
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Damaged archive: Redundant 'K' headers may cause linknames to be incorrect");
+                               err = err_combine(err, ARCHIVE_WARN);
                        }
                        seen_headers |= seen_K_header;
+                       a->archive.archive_format = ARCHIVE_FORMAT_TAR_GNUTAR;
+                       a->archive.archive_format_name = "GNU tar format";
                        err2 = header_gnu_longlink(a, tar, entry, h, unconsumed);
                        break;
                case 'L': /* Long filename (GNU tar, others) */
                        if (seen_headers & seen_L_header) {
-                               return (ARCHIVE_FATAL);
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Damaged archive: Redundant 'L' headers may cause filenames to be incorrect");
+                               err = err_combine(err, ARCHIVE_WARN);
                        }
                        seen_headers |= seen_L_header;
+                       a->archive.archive_format = ARCHIVE_FORMAT_TAR_GNUTAR;
+                       a->archive.archive_format_name = "GNU tar format";
                        err2 = header_gnu_longname(a, tar, entry, h, unconsumed);
                        break;
                case 'V': /* GNU volume header */
                        if (seen_headers & seen_V_header) {
-                               return (ARCHIVE_FATAL);
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Redundant 'V' header");
+                               err = err_combine(err, ARCHIVE_WARN);
                        }
                        seen_headers |= seen_V_header;
+                       a->archive.archive_format = ARCHIVE_FORMAT_TAR_GNUTAR;
+                       a->archive.archive_format_name = "GNU tar format";
                        err2 = header_volume(a, tar, entry, h, unconsumed);
                        break;
                case 'X': /* Used by SUN tar; same as 'x'. */
                        if (seen_headers & seen_x_header) {
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Redundant 'X'/'x' header");
                                return (ARCHIVE_FATAL);
                        }
                        seen_headers |= seen_x_header;
@@ -865,6 +883,8 @@ tar_read_header(struct archive_read *a, struct tar *tar,
                        break;
                case 'x': /* POSIX-standard 'x' header. */
                        if (seen_headers & seen_x_header) {
+                               archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+                                                 "Redundant 'x' header");
                                return (ARCHIVE_FATAL);
                        }
                        seen_headers |= seen_x_header;

diff --git a/libarchive/archive_write_set_format_gnutar.c b/libarchive/archive_write_set_format_gnutar.c
index ec1e1d342feab2ffdca2418ca4fd9f20de299154..14ef4cd2444d6a3c6d6e2aa1ae7e69490f9eab66 100644 (file)
--- a/libarchive/archive_write_set_format_gnutar.c
+++ b/libarchive/archive_write_set_format_gnutar.c
@@ -504,7 +504,7 @@ archive_write_gnutar_header(struct archive_write *a,
                archive_entry_set_uname(temp, "root");
                archive_entry_set_gname(temp, "wheel");
 
-               archive_entry_set_pathname(temp, "././@LongLink");
+               archive_entry_set_pathname(temp, "././@LongName");
                archive_entry_set_size(temp, length);
                ret = archive_format_gnutar_header(a, buff, temp, 'L');
                archive_entry_free(temp);
@@ -640,7 +640,7 @@ archive_format_gnutar_header(struct archive_write *a, char h[512],
        if (format_number(archive_entry_uid(entry), h + GNUTAR_uid_offset,
                GNUTAR_uid_size, GNUTAR_uid_max_size)) {
                archive_set_error(&a->archive, ERANGE,
-                   "Numeric user ID %jd too large",
+                   "Numeric user ID %jd too large for gnutar format",
                    (intmax_t)archive_entry_uid(entry));
                ret = ARCHIVE_FAILED;
        }
@@ -649,7 +649,7 @@ archive_format_gnutar_header(struct archive_write *a, char h[512],
        if (format_number(archive_entry_gid(entry), h + GNUTAR_gid_offset,
                GNUTAR_gid_size, GNUTAR_gid_max_size)) {
                archive_set_error(&a->archive, ERANGE,
-                   "Numeric group ID %jd too large",
+                   "Numeric group ID %jd too large for gnutar format",
                    (intmax_t)archive_entry_gid(entry));
                ret = ARCHIVE_FAILED;
        }
@@ -672,7 +672,7 @@ archive_format_gnutar_header(struct archive_write *a, char h[512],
                    h + GNUTAR_rdevmajor_offset,
                        GNUTAR_rdevmajor_size)) {
                        archive_set_error(&a->archive, ERANGE,
-                           "Major device number too large");
+                           "Major device number too large for gnutar format");
                        ret = ARCHIVE_FAILED;
                }
 
@@ -680,7 +680,7 @@ archive_format_gnutar_header(struct archive_write *a, char h[512],
                    h + GNUTAR_rdevminor_offset,
                        GNUTAR_rdevminor_size)) {
                        archive_set_error(&a->archive, ERANGE,
-                           "Minor device number too large");
+                           "Minor device number too large for gnutar format");
                        ret = ARCHIVE_FAILED;
                }
        }

diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt
index 991a6caee5b43aea800c8c884a1f92a883eaf01c..76009a1b1f45e381ba9a8eebea5d9a74cadcda31 100644 (file)
--- a/libarchive/test/CMakeLists.txt
+++ b/libarchive/test/CMakeLists.txt
@@ -133,6 +133,7 @@ IF(ENABLE_TEST)
     test_read_format_gtar_filename.c
     test_read_format_gtar_gz.c
     test_read_format_gtar_lzma.c
+    test_read_format_gtar_redundant_L.c
     test_read_format_gtar_sparse.c
     test_read_format_gtar_sparse_length.c
     test_read_format_gtar_sparse_skip_entry.c

diff --git a/libarchive/test/test_read_format_gtar_redundant_L.c b/libarchive/test/test_read_format_gtar_redundant_L.c
new file mode 100644 (file)
index 0000000..167445c
--- /dev/null
+++ b/libarchive/test/test_read_format_gtar_redundant_L.c
@@ -0,0 +1,40 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2024 Tim Kientzle
+ * All rights reserved.
+ */
+#include "test.h"
+
+#include <string.h>
+
+DEFINE_TEST(test_read_format_gtar_redundant_L)
+{
+       const char *refname = "test_read_format_gtar_redundant_L.tar.Z";
+       struct archive *a;
+       struct archive_entry *ae;
+
+       extract_reference_file(refname);
+       assert((a = archive_read_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a));
+       assertEqualIntA(a, ARCHIVE_OK,
+           archive_read_open_filename(a, refname, 10240));
+
+       /* First file has redundant 'L' headers; this should prompt
+        * a suitable ARCHIVE_WARN message */
+       assertEqualIntA(a, ARCHIVE_WARN, archive_read_next_header(a, &ae));
+       assertEqualInt(archive_errno(a), -1);
+       assert(strstr(archive_error_string(a), "Redundant 'L'") != NULL);
+
+       /* End of archive. */
+       assertEqualIntA(a, ARCHIVE_EOF, archive_read_next_header(a, &ae));
+
+       /* Verify archive format. */
+       assertEqualIntA(a, ARCHIVE_FILTER_COMPRESS, archive_filter_code(a, 0));
+       assertEqualIntA(a, ARCHIVE_FORMAT_TAR_GNUTAR, archive_format(a));
+
+       /* Close the archive. */
+       assertEqualInt(ARCHIVE_OK, archive_read_close(a));
+       assertEqualInt(ARCHIVE_OK, archive_read_free(a));
+}

diff --git a/libarchive/test/test_read_format_gtar_redundant_L.tar.Z.uu b/libarchive/test/test_read_format_gtar_redundant_L.tar.Z.uu
new file mode 100644 (file)
index 0000000..3a0e447
--- /dev/null
+++ b/libarchive/test/test_read_format_gtar_redundant_L.tar.Z.uu
@@ -0,0 +1,16 @@
+begin 644 test_read_format_gtar_redundant_L.tar.Z
+M'YV0+EX$!,+DC9LS3-*X60.@H<.'$"-*G$BQHL6+%&%HA&&#!@T`&T."#*EQ
+M)$D8,F#$,'FRI,H8-6Y\!,$$H\V;.'/JE%AG#ITP<D"``"#GS1LZ.RD6/9JT
+MJ=.G4*-*G?K4YQLY8<Z4>5&F31TV8>B4(?,"QHL@;L@434.63-@P+_#(G4NW
+MKMV[>//JW<NW;UVJ@`,+'DRXL.'#B!,W#3BPX,&$"Q5+GABRX\>6+#=FWACC
+MQLJ6(E_:0`F`YN33.7O^##IT*=*IKE'+GDV;L%6L6KEZ!2N6K%FT:M^P?>'V
+M9UR_R),K7WZWMO/GT*-+GWZ:\0N"!A$J9$A]<&6/FV&$#X]2)7G-+V/.K-E=
+MMFJ@0HD:?2TU=OO[^!7?SKJUZ]>P8Y5U5EIKM?76<<PEJ.""?.7GX(,01BCA
+M3=9A]]AV$SKUW64GC0=:9Y^!YE(,,8PF0VGL93C8>ZS)QQ1L\ZDHXXP2[9>;
+M?[P%^!N!PAEH'(-`!ID<C406:>21@Z%1!AMLO.$"'7C0AR1&*FED&4LWV%!#
+M9C)P"-H,`,1`PPTSS&"###74`,,-()%(`PPSB3?E4RS&1T<:;0#F4QAFF#'G
+MG]VAD88"@!9JZ*&()JKHHHPVZNBCD$8JZ:245FKII9AFJNFFG';JZ:>@ABKJ
+MJ*26:NJIJ*:JZJJLMNKJJ[#&*NNLM-9JZZVXYJKKKKSVZNNOP`8K[+#$%FOL
+3L<@FJ^RRS#;K[+/01BOMM-0N"@``
+`
+end

diff --git a/libarchive/test/test_read_format_tar_empty_with_gnulabel.c b/libarchive/test/test_read_format_tar_empty_with_gnulabel.c
index 3ff5b318d51af155212c6f2717c7a7f2af2870d6..a86bb287bf4e0ac6dc7375243d2949d3563cfae0 100644 (file)
--- a/libarchive/test/test_read_format_tar_empty_with_gnulabel.c
+++ b/libarchive/test/test_read_format_tar_empty_with_gnulabel.c
@@ -45,7 +45,7 @@ DEFINE_TEST(test_read_format_tar_empty_with_gnulabel)
 
        /* Verify that the format detection worked. */
        assertEqualInt(archive_filter_code(a, 0), ARCHIVE_FILTER_NONE);
-       assertEqualInt(archive_format(a), ARCHIVE_FORMAT_TAR);
+       assertEqualInt(archive_format(a), ARCHIVE_FORMAT_TAR_GNUTAR);
 
        assertEqualInt(ARCHIVE_OK, archive_read_close(a));
        assertEqualInt(ARCHIVE_OK, archive_read_free(a));

diff --git a/libarchive/test/test_write_format_gnutar.c b/libarchive/test/test_write_format_gnutar.c
index d72d076d115c40040f0324e20468379986a7cbad..ce963f43b66d39b4f0b442302f09888790d65028 100644 (file)
--- a/libarchive/test/test_write_format_gnutar.c
+++ b/libarchive/test/test_write_format_gnutar.c
@@ -93,6 +93,29 @@ static const char *longhardlinkname = "Yabcdefghijklmnopqrstuvwxyz"
     "12345678901234567890123456789012345678901234567890"
     "12345678901234567890123456789012345678901234567890";
 
+static const char *longfilename_largeuid = "large_uid_gid---"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890"
+    "12345678901234567890123456789012345678901234567890";
+
+
 
 DEFINE_TEST(test_write_format_gnutar)
 {
@@ -162,7 +185,7 @@ DEFINE_TEST(test_write_format_gnutar)
         * A file with large UID/GID that overflow octal encoding.
         */
        assert((ae = archive_entry_new()) != NULL);
-       archive_entry_copy_pathname(ae, "large_uid_gid");
+       archive_entry_copy_pathname(ae, longfilename_largeuid);
        archive_entry_set_mode(ae, S_IFREG | 0755);
        archive_entry_set_size(ae, 8);
        archive_entry_set_uid(ae, 123456789);
@@ -185,7 +208,7 @@ DEFINE_TEST(test_write_format_gnutar)
        /* Verify GNU tar magic/version fields */
        assertEqualMem(buff + 257, "ustar  \0", 8);
 
-       assertEqualInt(15360, used);
+       assertEqualInt(16896, used);
 
        /*
         *
@@ -243,7 +266,7 @@ DEFINE_TEST(test_write_format_gnutar)
        assertEqualIntA(a, 0, archive_read_next_header(a, &ae));
        assertEqualInt(123456789, archive_entry_uid(ae));
        assertEqualInt(987654321, archive_entry_gid(ae));
-       assertEqualString("large_uid_gid", archive_entry_pathname(ae));
+       assertEqualString(longfilename_largeuid, archive_entry_pathname(ae));
        assertEqualInt(S_IFREG | 0755, archive_entry_mode(ae));
 
        /*