Do not follow symlinks when processing the fixup list

Use lchmod() instead of chmod() and tell the remaining functions that the
real file to be modified is a symbolic link.

Fixes #1566

diff --git a/Makefile.am b/Makefile.am
index 58edb74e337bd4c8668246d420af3fc464963466..c93a82e922af4f61add50adf5ffe740e13471d9b 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -560,6 +560,7 @@ libarchive_test_SOURCES= \
        libarchive/test/test_write_disk.c \
        libarchive/test/test_write_disk_appledouble.c \
        libarchive/test/test_write_disk_failures.c \
+       libarchive/test/test_write_disk_fixup.c \
        libarchive/test/test_write_disk_hardlink.c \
        libarchive/test/test_write_disk_hfs_compression.c \
        libarchive/test/test_write_disk_lookup.c \

diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
index 8474617eb5ca98ce12f234990d72e6681ad490a8..fcd733aff51ee43ebff384f7fb56902138341701 100644 (file)
--- a/libarchive/archive_write_disk_posix.c
+++ b/libarchive/archive_write_disk_posix.c
@@ -2461,6 +2461,7 @@ _archive_write_disk_close(struct archive *_a)
 {
        struct archive_write_disk *a = (struct archive_write_disk *)_a;
        struct fixup_entry *next, *p;
+       struct stat st;
        int fd, ret;
 
        archive_check_magic(&a->archive, ARCHIVE_WRITE_DISK_MAGIC,
@@ -2478,6 +2479,20 @@ _archive_write_disk_close(struct archive *_a)
                    (TODO_TIMES | TODO_MODE_BASE | TODO_ACLS | TODO_FFLAGS)) {
                        fd = open(p->name,
                            O_WRONLY | O_BINARY | O_NOFOLLOW | O_CLOEXEC);
+                       if (fd == -1) {
+                               /* If we cannot lstat, skip entry */
+                               if (lstat(p->name, &st) != 0)
+                                       goto skip_fixup_entry;
+                               /*
+                                * If we deal with a symbolic link, mark
+                                * it in the fixup mode to ensure no
+                                * modifications are made to its target.
+                                */
+                               if (S_ISLNK(st.st_mode)) {
+                                       p->mode &= ~S_IFMT;
+                                       p->mode |= S_IFLNK;
+                               }
+                       }
                }
                if (p->fixup & TODO_TIMES) {
                        set_times(a, fd, p->mode, p->name,
@@ -2492,7 +2507,12 @@ _archive_write_disk_close(struct archive *_a)
                                fchmod(fd, p->mode);
                        else
 #endif
-                       chmod(p->name, p->mode);
+#ifdef HAVE_LCHMOD
+                       lchmod(p->name, p->mode);
+#else
+                       if (!S_ISLNK(p->mode))
+                               chmod(p->name, p->mode);
+#endif
                }
                if (p->fixup & TODO_ACLS)
                        archive_write_disk_set_acls(&a->archive, fd,
@@ -2503,6 +2523,7 @@ _archive_write_disk_close(struct archive *_a)
                if (p->fixup & TODO_MAC_METADATA)
                        set_mac_metadata(a, p->name, p->mac_metadata,
                                         p->mac_metadata_size);
+skip_fixup_entry:
                next = p->next;
                archive_acl_clear(&p->acl);
                free(p->mac_metadata);
@@ -2643,6 +2664,7 @@ new_fixup(struct archive_write_disk *a, const char *pathname)
        fe->next = a->fixup_list;
        a->fixup_list = fe;
        fe->fixup = 0;
+       fe->mode = 0;
        fe->name = strdup(pathname);
        return (fe);
 }

diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt
index b26f679cf064fa88c9ba63cdb850709030d9f3d2..53cc3e2256779b9eb1ba1135b1a202925a1795cd 100644 (file)
--- a/libarchive/test/CMakeLists.txt
+++ b/libarchive/test/CMakeLists.txt
@@ -209,6 +209,7 @@ IF(ENABLE_TEST)
     test_write_disk.c
     test_write_disk_appledouble.c
     test_write_disk_failures.c
+    test_write_disk_fixup.c
     test_write_disk_hardlink.c
     test_write_disk_hfs_compression.c
     test_write_disk_lookup.c

diff --git a/libarchive/test/test_write_disk_fixup.c b/libarchive/test/test_write_disk_fixup.c
new file mode 100644 (file)
index 0000000..153cc3a
--- /dev/null
+++ b/libarchive/test/test_write_disk_fixup.c
@@ -0,0 +1,77 @@
+/*-
+ * Copyright (c) 2021 Martin Matuska
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include "test.h"
+
+/*
+ * Test fixup entries don't follow symlinks
+ */
+DEFINE_TEST(test_write_disk_fixup)
+{
+       struct archive *ad;
+       struct archive_entry *ae;
+       int r;
+
+       if (!canSymlink()) {
+               skipping("Symlinks not supported");
+               return;
+       }
+
+       /* Write entries to disk. */
+       assert((ad = archive_write_disk_new()) != NULL);
+
+       /*
+        * Create a file
+        */
+       assertMakeFile("victim", 0600, "a");
+
+       /*
+        * Create a directory and a symlink with the same name
+        */
+
+       /* Directory: dir */
+        assert((ae = archive_entry_new()) != NULL);
+        archive_entry_copy_pathname(ae, "dir");
+        archive_entry_set_mode(ae, AE_IFDIR | 0606);
+       assertEqualIntA(ad, 0, archive_write_header(ad, ae));
+       assertEqualIntA(ad, 0, archive_write_finish_entry(ad));
+        archive_entry_free(ae);
+
+       /* Symbolic Link: dir -> foo */
+       assert((ae = archive_entry_new()) != NULL);
+       archive_entry_copy_pathname(ae, "dir");
+       archive_entry_set_mode(ae, AE_IFLNK | 0777);
+       archive_entry_set_size(ae, 0);
+       archive_entry_copy_symlink(ae, "victim");
+       assertEqualIntA(ad, 0, r = archive_write_header(ad, ae));
+       if (r >= ARCHIVE_WARN)
+               assertEqualIntA(ad, 0, archive_write_finish_entry(ad));
+       archive_entry_free(ae);
+
+       assertEqualInt(ARCHIVE_OK, archive_write_free(ad));
+
+       /* Test the entries on disk. */
+       assertIsSymlink("dir", "victim", 0);
+       assertFileMode("victim", 0600);
+}