Fix replacing a regular file with a dir for ARCHIVE_EXTRACT_SAFE_WRITES

The outer if checks !S_ISDIR(a->st.st_mode), so we know that the file
being overwritten is not a directory, and thus we can rename(2) over it
if we want to, but whether we can use a temporary regular file is a
property of the file being extracted. Otherwise, when replacing a
regular file with a directory, we end up in this case and create a
temporary regular file for the new directory, but with the permissions
of the directory (which likely includes x), and rename it over the top
at the end. Depending on where the archive_entry came from, it may have
a non-zero size that also isn't ovewritten with 0 (e.g. if it came from
stat(2)) and so the API user may then try to copy data (thus failing if
read(2) of directories isn't permitted, or writing the raw directory
contents if it is), but if the size is zero as is the case for this tar
test then it will end up not writing any data and "successfully"
overwrite the file with an empty file, not a directory.

diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
index 6b6ded739279bf39abbf5d73714f69da89af3552..443d88e7d1234fe7cd6fe546e3ce365d692995fc 100644 (file)
--- a/libarchive/archive_write_disk_posix.c
+++ b/libarchive/archive_write_disk_posix.c
@@ -2202,7 +2202,7 @@ restore_entry(struct archive_write_disk *a)
                                (void)clear_nochange_fflags(a);
 
                        if ((a->flags & ARCHIVE_EXTRACT_SAFE_WRITES) &&
-                           S_ISREG(a->st.st_mode)) {
+                           S_ISREG(a->mode)) {
                                /* Use a temporary file to extract */
                                if ((a->fd = la_mktemp(a)) == -1) {
                                        archive_set_error(&a->archive, errno,

diff --git a/tar/test/test_option_safe_writes.c b/tar/test/test_option_safe_writes.c
index b88479bc5f35a929b9ce4c1f7f51c9a3df362866..d30b9a7459278ebd35921cf6000d85af64e14107 100644 (file)
--- a/tar/test/test_option_safe_writes.c
+++ b/tar/test/test_option_safe_writes.c
@@ -16,11 +16,12 @@ DEFINE_TEST(test_option_safe_writes)
        assertMakeFile("d", 0644, "c");
        assertMakeFile("fs", 0644, "d");
        assertMakeFile("ds", 0644, "e");
+       assertMakeDir("fd", 0755);
        assertEqualInt(0, chdir(".."));
 
        /* Tar files up */
        assertEqualInt(0,
-           systemf("%s -c -C in -f t.tar f fh d fs ds "
+           systemf("%s -c -C in -f t.tar f fh d fs ds fd "
            ">pack.out 2>pack.err", testprog));
 
         /* Verify that nothing went to stdout or stderr. */
@@ -32,6 +33,7 @@ DEFINE_TEST(test_option_safe_writes)
        assertEqualInt(0, chdir("out"));
        assertMakeFile("f", 0644, "a");
        assertMakeHardlink("fh", "f");
+       assertMakeFile("fd", 0644, "b");
        assertMakeDir("d", 0755);
        if (canSymlink()) {
                assertMakeSymlink("fs", "f", 0);
@@ -55,4 +57,5 @@ DEFINE_TEST(test_option_safe_writes)
        assertTextFileContents("c","d");
        assertTextFileContents("d","fs");
        assertTextFileContents("e","ds");
+       assertIsDir("fd", 0755);
 }