On Tue, Mar 3, 2009 at 9:08 AM, Jim Meyering <j...@meyering.net> wrote:
>
> Thanks for the report.  I read Casper's reply.
>
> That function removes PRIV_SYS_LINKDIR for the sake of security.
> Without it, there's a guaranteed race condition that may lead to
> unlinking a non-empty directory.
>
> It's ironic that Solaris' privilege system does not accommodate that.
> But if you explain to them why we're doing it, maybe they'll
> recognize the utility of it and relax the model.
>

Hi Jim,

I've confirmed that this is also a problem on Solaris 10, so even if
Sun were to agree to change it there would still be many systems
affected.

How does the attached patch look? The idea is to restore the
PRIV_SYS_LINKDIR privilege if chmod fails and try the chmod again. If
it looks ok, I'll split the patch into separate gnulib and tar patches
and add a changelog entry.

-- David
diff -uraN tar-1.21.orig/lib/unlinkdir.c tar-1.21/lib/unlinkdir.c
--- tar-1.21.orig/lib/unlinkdir.c	2007-11-18 05:46:57.000000000 -0500
+++ tar-1.21/lib/unlinkdir.c	2009-03-04 04:25:16.277473318 -0500
@@ -1,6 +1,6 @@
 /* unlinkdir.c - determine (and maybe change) whether we can unlink directories
 
-   Copyright (C) 2005, 2006 Free Software Foundation, Inc.
+   Copyright (C) 2005, 2006, 2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -15,7 +15,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Written by Paul Eggert and Jim Meyering.  */
+/* Written by Paul Eggert, Jim Meyering, and David Bartley.  */
 
 #include <config.h>
 
@@ -28,6 +28,11 @@
 
 #if ! UNLINK_CANNOT_UNLINK_DIR
 
+static bool initialized;
+# if defined PRIV_EFFECTIVE && defined PRIV_SYS_LINKDIR
+static bool had_sys_linkdir;
+#endif
+
 /* Return true if we cannot unlink directories, false if we might be
    able to unlink directories.  If possible, tell the kernel we don't
    want to be able to unlink directories, so that we can return true.  */
@@ -35,7 +40,6 @@
 bool
 cannot_unlink_dir (void)
 {
-  static bool initialized;
   static bool cannot;
 
   if (! initialized)
@@ -47,11 +51,15 @@
       priv_set_t *pset = priv_allocset ();
       if (pset)
 	{
-	  cannot =
-	    (getppriv (PRIV_EFFECTIVE, pset) == 0
-	     && (! priv_ismember (pset, PRIV_SYS_LINKDIR)
-		 || (priv_delset (pset, PRIV_SYS_LINKDIR) == 0
-		     && setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0)));
+	  if (getppriv (PRIV_EFFECTIVE, pset) == 0)
+	    {
+	      if (priv_ismember (pset, PRIV_SYS_LINKDIR))
+		cannot = had_sys_linkdir =
+		  (priv_delset (pset, PRIV_SYS_LINKDIR) == 0
+		  && setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0);
+	      else
+		cannot = true;
+	    }
 	  priv_freeset (pset);
 	}
 # else
@@ -64,4 +72,38 @@
   return cannot;
 }
 
+
+/* Return true if the kernel restored the ability of unlink to remove
+   directories.  */
+
+bool
+restore_unlink_dir (void)
+{
+  bool restored = false;
+
+# if defined PRIV_EFFECTIVE && defined PRIV_SYS_LINKDIR
+  if (initialized)
+    {
+      /* Try to re-add the PRIV_SYS_LINKDIR privilege, but only if we
+	 previously removed it.  */
+      if (had_sys_linkdir)
+        {
+	  priv_set_t *pset = priv_allocset ();
+	  if (pset)
+	    {
+	      if (getppriv (PRIV_EFFECTIVE, pset) == 0)
+		{
+		  initialized = ! (priv_addset (pset, PRIV_SYS_LINKDIR) == 0
+		    && setppriv (PRIV_SET, PRIV_EFFECTIVE, pset) == 0);
+		}
+	    }
+	  priv_freeset (pset);
+	  restored = ! initialized;
+	}
+    }
+# endif
+
+  return restored;
+}
+
 #endif
diff -uraN tar-1.21.orig/lib/unlinkdir.h tar-1.21/lib/unlinkdir.h
--- tar-1.21.orig/lib/unlinkdir.h	2007-11-18 05:46:57.000000000 -0500
+++ tar-1.21/lib/unlinkdir.h	2009-03-04 04:25:16.277618403 -0500
@@ -1,6 +1,6 @@
 /* unlinkdir.h - determine (and maybe change) whether we can unlink directories
 
-   Copyright (C) 2005 Free Software Foundation, Inc.
+   Copyright (C) 2005, 2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -15,12 +15,14 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Written by Paul Eggert and Jim Meyering.  */
+/* Written by Paul Eggert, Jim Meyering, and David Bartley.  */
 
 #include <stdbool.h>
 
 #if UNLINK_CANNOT_UNLINK_DIR
 # define cannot_unlink_dir() true
+# define restore_unlink_dir() false
 #else
 bool cannot_unlink_dir (void);
+bool restore_unlink_dir (void);
 #endif
diff -uraN tar-1.21.orig/src/extract.c tar-1.21/src/extract.c
--- tar-1.21.orig/src/extract.c	2008-10-30 10:10:28.000000000 -0400
+++ tar-1.21/src/extract.c	2009-03-04 04:25:26.119402470 -0500
@@ -1,7 +1,7 @@
 /* Extract files from a tar archive.
 
    Copyright (C) 1988, 1992, 1993, 1994, 1996, 1997, 1998, 1999, 2000,
-   2001, 2003, 2004, 2005, 2006, 2007 Free Software Foundation, Inc.
+   2001, 2003, 2004, 2005, 2006, 2007, 2009 Free Software Foundation, Inc.
 
    Written by John Gilmore, on 1985-11-19.
 
@@ -24,6 +24,7 @@
 #include <utimens.h>
 #include <errno.h>
 #include <xgetcwd.h>
+#include <unlinkdir.h>
 
 #include "common.h"
 
@@ -186,7 +187,15 @@
       mode = cur_info->st_mode ^ invert_permissions;
     }
 
-  if (chmod (file_name, mode) != 0)
+  bool failed = (chmod (file_name, mode) != 0);
+  if (failed && errno == EPERM)
+    {
+      /* On Solaris, chmod may fail if we dropped a privilege (this happens
+         when we call cannot_unlink_dir).  */
+      if (restore_unlink_dir ())
+        failed = (chmod (file_name, mode) != 0);
+    }
+  if (failed)
     chmod_error_details (file_name, mode);
 }
 

Reply via email to