|
|
From 14d8fc718f0c872274b90991ee634b0cd8e1a6f0 Mon Sep 17 00:00:00 2001
|
|
|
From: Sergey Poznyakoff <gray@gnu.org>
|
|
|
Date: Sat, 8 Feb 2020 13:01:47 +0200
|
|
|
Subject: [PATCH] Fix the --no-overwrite-dir option
|
|
|
|
|
|
Given this option, tar failed to preserve permissions of empty directories
|
|
|
and to create files under directories owned by the current user that did
|
|
|
not have the S_IWUSR bit set.
|
|
|
|
|
|
* src/extract.c (fd_chmod): Rename to fd_i_chmod.
|
|
|
(fd_chmod): New function.
|
|
|
(safe_dir_mode): New function.
|
|
|
(extract_dir): Special handling for existing directories in
|
|
|
--no-overwrite-dir mode.
|
|
|
* tests/extrac23.at: New file.
|
|
|
* tests/Makefile.am: Add new test case.
|
|
|
* tests/testsuite.at: Likewise.
|
|
|
---
|
|
|
src/extract.c | 128 ++++++++++++++++++++++++++++++---------------
|
|
|
tests/Makefile.am | 1 +
|
|
|
tests/extrac23.at | 58 ++++++++++++++++++++
|
|
|
tests/testsuite.at | 1 +
|
|
|
4 files changed, 146 insertions(+), 42 deletions(-)
|
|
|
create mode 100644 tests/extrac23.at
|
|
|
|
|
|
diff --git a/src/extract.c b/src/extract.c
|
|
|
index a4a35a57..5a38ba70 100644
|
|
|
--- a/src/extract.c
|
|
|
+++ b/src/extract.c
|
|
|
@@ -194,7 +194,7 @@ extr_init (void)
|
|
|
|
|
|
/* Use fchmod if possible, fchmodat otherwise. */
|
|
|
static int
|
|
|
-fd_chmod (int fd, char const *file, mode_t mode, int atflag)
|
|
|
+fd_i_chmod (int fd, char const *file, mode_t mode, int atflag)
|
|
|
{
|
|
|
if (0 <= fd)
|
|
|
{
|
|
|
@@ -205,6 +205,42 @@ fd_chmod (int fd, char const *file, mode_t mode, int atflag)
|
|
|
return fchmodat (chdir_fd, file, mode, atflag);
|
|
|
}
|
|
|
|
|
|
+/* A version of fd_i_chmod which gracefully handles several common error
|
|
|
+ conditions. Additional argument TYPEFLAG is the type of file in tar
|
|
|
+ notation.
|
|
|
+ */
|
|
|
+static int
|
|
|
+fd_chmod(int fd, char const *file_name, int mode, int atflag, int typeflag)
|
|
|
+{
|
|
|
+ int chmod_errno = fd_i_chmod (fd, file_name, mode, atflag) == 0 ? 0 : errno;
|
|
|
+
|
|
|
+ /* On Solaris, chmod may fail if we don't have PRIV_ALL, because
|
|
|
+ setuid-root files would otherwise be a backdoor. See
|
|
|
+ http://opensolaris.org/jive/thread.jspa?threadID=95826
|
|
|
+ (2009-09-03). */
|
|
|
+ if (chmod_errno == EPERM && (mode & S_ISUID)
|
|
|
+ && priv_set_restore_linkdir () == 0)
|
|
|
+ {
|
|
|
+ chmod_errno = fd_i_chmod (fd, file_name, mode, atflag) == 0 ? 0 : errno;
|
|
|
+ priv_set_remove_linkdir ();
|
|
|
+ }
|
|
|
+
|
|
|
+ /* Linux fchmodat does not support AT_SYMLINK_NOFOLLOW, and
|
|
|
+ returns ENOTSUP even when operating on non-symlinks, try
|
|
|
+ again with the flag disabled if it does not appear to be
|
|
|
+ supported and if the file is not a symlink. This
|
|
|
+ introduces a race, alas. */
|
|
|
+ if (atflag && typeflag != SYMTYPE && ! implemented (chmod_errno))
|
|
|
+ chmod_errno = fd_i_chmod (fd, file_name, mode, 0) == 0 ? 0 : errno;
|
|
|
+
|
|
|
+ if (chmod_errno && (typeflag != SYMTYPE || implemented (chmod_errno)))
|
|
|
+ {
|
|
|
+ errno = chmod_errno;
|
|
|
+ return -1;
|
|
|
+ }
|
|
|
+ return 0;
|
|
|
+}
|
|
|
+
|
|
|
/* Use fchown if possible, fchownat otherwise. */
|
|
|
static int
|
|
|
fd_chown (int fd, char const *file, uid_t uid, gid_t gid, int atflag)
|
|
|
@@ -259,35 +295,8 @@ set_mode (char const *file_name,
|
|
|
|
|
|
if (current_mode != mode)
|
|
|
{
|
|
|
- int chmod_errno =
|
|
|
- fd_chmod (fd, file_name, mode, atflag) == 0 ? 0 : errno;
|
|
|
-
|
|
|
- /* On Solaris, chmod may fail if we don't have PRIV_ALL, because
|
|
|
- setuid-root files would otherwise be a backdoor. See
|
|
|
- http://opensolaris.org/jive/thread.jspa?threadID=95826
|
|
|
- (2009-09-03). */
|
|
|
- if (chmod_errno == EPERM && (mode & S_ISUID)
|
|
|
- && priv_set_restore_linkdir () == 0)
|
|
|
- {
|
|
|
- chmod_errno =
|
|
|
- fd_chmod (fd, file_name, mode, atflag) == 0 ? 0 : errno;
|
|
|
- priv_set_remove_linkdir ();
|
|
|
- }
|
|
|
-
|
|
|
- /* Linux fchmodat does not support AT_SYMLINK_NOFOLLOW, and
|
|
|
- returns ENOTSUP even when operating on non-symlinks, try
|
|
|
- again with the flag disabled if it does not appear to be
|
|
|
- supported and if the file is not a symlink. This
|
|
|
- introduces a race, alas. */
|
|
|
- if (atflag && typeflag != SYMTYPE && ! implemented (chmod_errno))
|
|
|
- chmod_errno = fd_chmod (fd, file_name, mode, 0) == 0 ? 0 : errno;
|
|
|
-
|
|
|
- if (chmod_errno
|
|
|
- && (typeflag != SYMTYPE || implemented (chmod_errno)))
|
|
|
- {
|
|
|
- errno = chmod_errno;
|
|
|
- chmod_error_details (file_name, mode);
|
|
|
- }
|
|
|
+ if (fd_chmod (fd, file_name, mode, atflag, typeflag))
|
|
|
+ chmod_error_details (file_name, mode);
|
|
|
}
|
|
|
}
|
|
|
}
|
|
|
@@ -975,6 +984,26 @@ is_directory_link (const char *file_name)
|
|
|
return res;
|
|
|
}
|
|
|
|
|
|
+/* Given struct stat of a directory (or directory member) whose ownership
|
|
|
+ or permissions of will be restored later, return the temporary permissions
|
|
|
+ for that directory, sufficiently restrictive so that in the meantime
|
|
|
+ processes owned by other users do not inadvertently create files under this
|
|
|
+ directory that inherit the wrong owner, group, or permissions from the
|
|
|
+ directory.
|
|
|
+
|
|
|
+ If not root, though, make the directory writeable and searchable at first,
|
|
|
+ so that files can be created under it.
|
|
|
+*/
|
|
|
+static inline int
|
|
|
+safe_dir_mode (struct stat const *st)
|
|
|
+{
|
|
|
+ return ((st->st_mode
|
|
|
+ & (0 < same_owner_option || 0 < same_permissions_option
|
|
|
+ ? S_IRWXU
|
|
|
+ : MODE_RWX))
|
|
|
+ | (we_are_root ? 0 : MODE_WXUSR));
|
|
|
+}
|
|
|
+
|
|
|
/* Extractor functions for various member types */
|
|
|
|
|
|
static int
|
|
|
@@ -1004,18 +1033,7 @@ extract_dir (char *file_name, int typeflag)
|
|
|
else if (typeflag == GNUTYPE_DUMPDIR)
|
|
|
skip_member ();
|
|
|
|
|
|
- /* If ownership or permissions will be restored later, create the
|
|
|
- directory with restrictive permissions at first, so that in the
|
|
|
- meantime processes owned by other users do not inadvertently
|
|
|
- create files under this directory that inherit the wrong owner,
|
|
|
- group, or permissions from the directory. If not root, though,
|
|
|
- make the directory writeable and searchable at first, so that
|
|
|
- files can be created under it. */
|
|
|
- mode = ((current_stat_info.stat.st_mode
|
|
|
- & (0 < same_owner_option || 0 < same_permissions_option
|
|
|
- ? S_IRWXU
|
|
|
- : MODE_RWX))
|
|
|
- | (we_are_root ? 0 : MODE_WXUSR));
|
|
|
+ mode = safe_dir_mode (¤t_stat_info.stat);
|
|
|
|
|
|
for (;;)
|
|
|
{
|
|
|
@@ -1031,6 +1049,7 @@ extract_dir (char *file_name, int typeflag)
|
|
|
if (errno == EEXIST
|
|
|
&& (interdir_made
|
|
|
|| keep_directory_symlink_option
|
|
|
+ || old_files_option == NO_OVERWRITE_DIR_OLD_FILES
|
|
|
|| old_files_option == DEFAULT_OLD_FILES
|
|
|
|| old_files_option == OVERWRITE_OLD_FILES))
|
|
|
{
|
|
|
@@ -1051,6 +1070,31 @@ extract_dir (char *file_name, int typeflag)
|
|
|
repair_delayed_set_stat (file_name, &st);
|
|
|
return 0;
|
|
|
}
|
|
|
+ else if (old_files_option == NO_OVERWRITE_DIR_OLD_FILES)
|
|
|
+ {
|
|
|
+ /* Temporarily change the directory mode to a safe
|
|
|
+ value, to be able to create files in it, should
|
|
|
+ the need be.
|
|
|
+ */
|
|
|
+ mode = safe_dir_mode (&st);
|
|
|
+ status = fd_chmod(-1, file_name, mode,
|
|
|
+ AT_SYMLINK_NOFOLLOW, DIRTYPE);
|
|
|
+ if (status == 0)
|
|
|
+ {
|
|
|
+ /* Store the actual directory mode, to be restored
|
|
|
+ later.
|
|
|
+ */
|
|
|
+ current_stat_info.stat = st;
|
|
|
+ current_mode = mode & ~ current_umask;
|
|
|
+ current_mode_mask = MODE_RWX;
|
|
|
+ atflag = AT_SYMLINK_NOFOLLOW;
|
|
|
+ break;
|
|
|
+ }
|
|
|
+ else
|
|
|
+ {
|
|
|
+ chmod_error_details (file_name, mode);
|
|
|
+ }
|
|
|
+ }
|
|
|
break;
|
|
|
}
|
|
|
}
|
|
|
diff --git a/tests/Makefile.am b/tests/Makefile.am
|
|
|
index 0369a950..31ae3460 100644
|
|
|
--- a/tests/Makefile.am
|
|
|
+++ b/tests/Makefile.am
|
|
|
@@ -121,6 +121,7 @@ TESTSUITE_AT = \
|
|
|
extrac19.at\
|
|
|
extrac20.at\
|
|
|
extrac21.at\
|
|
|
+ extrac23.at\
|
|
|
filerem01.at\
|
|
|
filerem02.at\
|
|
|
dirrem01.at\
|
|
|
diff --git a/tests/extrac23.at b/tests/extrac23.at
|
|
|
new file mode 100644
|
|
|
index 00000000..669d18b6
|
|
|
--- /dev/null
|
|
|
+++ b/tests/extrac23.at
|
|
|
@@ -0,0 +1,58 @@
|
|
|
+# Test suite for GNU tar. -*- Autotest -*-
|
|
|
+# Copyright 2020 Free Software Foundation, Inc.
|
|
|
+#
|
|
|
+# This file is part of GNU tar.
|
|
|
+#
|
|
|
+# GNU tar is free software; you can redistribute it and/or modify
|
|
|
+# it under the terms of the GNU General Public License as published by
|
|
|
+# the Free Software Foundation; either version 3 of the License, or
|
|
|
+# (at your option) any later version.
|
|
|
+#
|
|
|
+# GNU tar is distributed in the hope that it will be useful,
|
|
|
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
|
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
|
+# GNU General Public License for more details.
|
|
|
+#
|
|
|
+# You should have received a copy of the GNU General Public License
|
|
|
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
|
|
+AT_SETUP([--no-overwrite-dir])
|
|
|
+AT_KEYWORDS([extract extrac23 no-overwrite-dir])
|
|
|
+
|
|
|
+# Description: Implementation of the --no-overwrite-dir option was flawed in
|
|
|
+# tar versions up to 1.32.90. This option is intended to preserve metadata
|
|
|
+# of existing directories. In fact it worked only for non-empty directories.
|
|
|
+# Moreover, if the actual directory was owned by the user tar runs as and the
|
|
|
+# S_IWUSR bit was not set in its actual permissions, tar failed to create files
|
|
|
+# in it.
|
|
|
+#
|
|
|
+# Reported by: Michael Kaufmann <mail@michael-kaufmann.ch>
|
|
|
+# References: <20200207112934.Horde.anXzYhAj2CHiwUrw5CuT0G-@webmail.michael-kaufmann.ch>,
|
|
|
+# https://lists.gnu.org/archive/html/bug-tar/2020-02/msg00003.html
|
|
|
+
|
|
|
+AT_TAR_CHECK([
|
|
|
+# Test if the directory permissions are restored properly.
|
|
|
+mkdir dir
|
|
|
+chmod 755 dir
|
|
|
+tar cf a.tar dir
|
|
|
+chmod 777 dir
|
|
|
+tar -xf a.tar --no-overwrite-dir
|
|
|
+genfile --stat=mode.777 dir
|
|
|
+
|
|
|
+# Test if temprorary permissions are set correctly to allow the owner
|
|
|
+# to write to the directory.
|
|
|
+genfile --file dir/file
|
|
|
+tar cf a.tar dir
|
|
|
+rm dir/file
|
|
|
+chmod 400 dir
|
|
|
+tar -xf a.tar --no-overwrite-dir
|
|
|
+genfile --stat=mode.777 dir
|
|
|
+chmod 700 dir
|
|
|
+find dir
|
|
|
+],
|
|
|
+[0],
|
|
|
+[777
|
|
|
+400
|
|
|
+dir
|
|
|
+dir/file
|
|
|
+])
|
|
|
+AT_CLEANUP
|
|
|
diff --git a/tests/testsuite.at b/tests/testsuite.at
|
|
|
index 2cc43a19..0620a3c7 100644
|
|
|
--- a/tests/testsuite.at
|
|
|
+++ b/tests/testsuite.at
|
|
|
@@ -343,6 +343,7 @@ m4_include([extrac19.at])
|
|
|
m4_include([extrac19.at])
|
|
|
m4_include([extrac20.at])
|
|
|
m4_include([extrac21.at])
|
|
|
+m4_include([extrac23.at])
|
|
|
|
|
|
m4_include([backup01.at])
|
|
|
|
|
|
--
|
|
|
2.37.3
|
|
|
|
