Guard against the POSIX allowed behavior where access (file, X_OK)

Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>

        * glib/gfileutils.c (g_file_test): Guard against
        the POSIX allowed behavior where access (file, X_OK)
        succeeds for uid==0 when no executable bits are set.

        * glib/gfileutils.c (g_file_test): Add doc notes
        about the possibility of race conditions, and the
        fact that EXISTS and IS_EXECUTABLE give results
        for the real uid not the effective user ID.
        (#81854, Morten Welinder.)

8 files changed, 134 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/ChangeLog.pre-2-10 b/ChangeLog.pre-2-10
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog.pre-2-10
+++ b/ChangeLog.pre-2-10
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/ChangeLog.pre-2-12 b/ChangeLog.pre-2-12
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog.pre-2-12
+++ b/ChangeLog.pre-2-12
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/ChangeLog.pre-2-2 b/ChangeLog.pre-2-2
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog.pre-2-2
+++ b/ChangeLog.pre-2-2
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/ChangeLog.pre-2-4 b/ChangeLog.pre-2-4
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog.pre-2-4
+++ b/ChangeLog.pre-2-4
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/ChangeLog.pre-2-6 b/ChangeLog.pre-2-6
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog.pre-2-6
+++ b/ChangeLog.pre-2-6
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/ChangeLog.pre-2-8 b/ChangeLog.pre-2-8
index e040a6dab..2bcf7ca94 100644
--- a/ChangeLog.pre-2-8
+++ b/ChangeLog.pre-2-8
@@ -1,3 +1,15 @@
+Sun Dec  8 23:36:12 2002  Owen Taylor  <otaylor@redhat.com>
+
+	* glib/gfileutils.c (g_file_test): Guard against
+	the POSIX allowed behavior where access (file, X_OK)
+	succeeds for uid==0 when no executable bits are set.
+
+	* glib/gfileutils.c (g_file_test): Add doc notes
+	about the possibility of race conditions, and the
+	fact that EXISTS and IS_EXECUTABLE give results
+	for the real uid not the effective user ID.
+	(#81854, Morten Welinder.)
+
 Fri Dec  6 14:34:42 2002  Owen Taylor  <otaylor@redhat.com>
 
 	Avoid literal UTF-8. (Allow it to build with 
diff --git a/glib/gfileutils.c b/glib/gfileutils.c
index 026db97f6..c67d5b5ef 100644
--- a/glib/gfileutils.c
+++ b/glib/gfileutils.c
@@ -77,12 +77,34 @@
  * test is %TRUE. With the current set of available tests, there's no point
  * passing in more than one test at a time.
  * 
- * Apart from #G_FILE_TEST_IS_SYMLINK all tests follow symbolic links,
+ * Apart from %G_FILE_TEST_IS_SYMLINK all tests follow symbolic links,
  * so for a symbolic link to a regular file g_file_test() will return
- * %TRUE for both #G_FILE_TEST_IS_SYMLINK and #G_FILE_TEST_IS_REGULAR.
+ * %TRUE for both %G_FILE_TEST_IS_SYMLINK and %G_FILE_TEST_IS_REGULAR.
  *
  * Note, that for a dangling symbolic link g_file_test() will return
- * %TRUE for #G_FILE_TEST_IS_SYMLINK and %FALSE for all other flags.
+ * %TRUE for %G_FILE_TEST_IS_SYMLINK and %FALSE for all other flags.
+ *
+ * You should never use g_file_test() to test whether it is safe
+ * to perform an operaton, because there is always the possibility
+ * of the condition changing before you actually perform the operation.
+ * For example, you might think you could use %G_FILE_TEST_IS_SYMLINK
+ * to know whether it is is safe to write to a file without being
+ * tricked into writing into a different location. It doesn't work!
+ *
+ * <informalexample><programlisting>
+ * /&ast; DON'T DO THIS &ast;/
+ *  if (!g_file_test (filename, G_FILE_TEST_IS_SYMLINK)) {
+ *    fd = open (filename, O_WRONLY);
+ *    /&ast; write to fd &ast;/
+ *  }
+ * </programlisting></informalexample>
+ *
+ * Another thing to note is that %G_FILE_TEST_EXISTS and
+ * %G_FILE_TEST_IS_EXECUTABLE are implemented using the access()
+ * system call. This usually doesn't matter, but if your program
+ * is setuid or setgid it means that these tests will give you
+ * the answer for the real user ID and group ID , rather than the
+ * effective user ID and group ID.
  *
  * Return value: whether a test was %TRUE
  **/
@@ -94,7 +116,19 @@ g_file_test (const gchar *filename,
     return TRUE;
   
   if ((test & G_FILE_TEST_IS_EXECUTABLE) && (access (filename, X_OK) == 0))
-    return TRUE;
+    {
+#ifndef G_OS_WIN32
+      if (getuid () != 0)
+#endif	
+	return TRUE;
+
+      /* For root, on some POSIX systems, access (filename, X_OK)
+       * will succeed even if no executable bits are set on the
+       * file. We fall through to a stat test to avoid that.
+       */
+    }
+  else
+    test &= ~G_FILE_TEST_IS_EXECUTABLE;
 
   if (test & G_FILE_TEST_IS_SYMLINK)
     {
@@ -108,7 +142,9 @@ g_file_test (const gchar *filename,
 #endif
     }
   
-  if (test & (G_FILE_TEST_IS_REGULAR | G_FILE_TEST_IS_DIR))
+  if (test & (G_FILE_TEST_IS_REGULAR |
+	      G_FILE_TEST_IS_DIR |
+	      G_FILE_TEST_IS_EXECUTABLE))
     {
       struct stat s;
       
@@ -119,6 +155,15 @@ g_file_test (const gchar *filename,
 	  
 	  if ((test & G_FILE_TEST_IS_DIR) && S_ISDIR (s.st_mode))
 	    return TRUE;
+
+	  /* The extra test for root when access (file, X_OK) succeeds.
+	   */
+	  if ((test & G_FILE_TEST_IS_EXECUTABLE) &&
+	      ((s.st_mode & S_IXOTH) ||
+	       (s.st_mode & S_IXUSR) ||
+	       (s.st_mode & S_IXGRP)))
+	    return TRUE;
+	      
 	}
     }