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 (&current_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