[newlib-cygwin] Disallow S_ISGID on directories without default ACL entries

Corinna Vinschen corinna@sourceware.org
Tue Apr 19 08:24:00 GMT 2016


commit 7a5b4524431110fde4e9336f64ade73ab2c26b6b
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Tue Apr 19 10:23:49 2016 +0200

    Disallow S_ISGID on directories without default ACL entries
    We can't handle the S_ISGID bit if the child didn't inherit a NULL SID
    ACE with the S_ISGID bit set.  On directories without default ACL
    entries we would have to add an inheritable NULL SID ACE and nothing else.
    This in turn results in permission problems when calling set_file_sd
    from set_created_file_access.  That's fixable, but it would only work
    for files created from Cygwin while files created from native Windows
    tools end up with really ugly permissions.
    This patch only makes sure that the S_ISGID bit is reset for a directory
    if it has no inheritable ACEs.  Still having the 's' bit shown in ls or
    getfacl output would be misleading.  So, calling `setfacl -k' on a dir
    also removes the S_ISGID bit now.
    	* sec_acl.cc (set_posix_access): Drop S_ISGID bit on directories
    	without inheritable ACEs.  Explain why.
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

 winsup/cygwin/sec_acl.cc | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/sec_acl.cc b/winsup/cygwin/sec_acl.cc
index 8dd73b19..cf0f89d 100644
--- a/winsup/cygwin/sec_acl.cc
+++ b/winsup/cygwin/sec_acl.cc
@@ -334,7 +334,19 @@ set_posix_access (mode_t attr, uid_t uid, gid_t gid,
       other_obj = aclbufp[tmp_idx].a_perm;
       /* ... class_obj.  Create NULL deny ACE.  Only the S_ISGID attribute gets
-	 inherited. */
+	 inherited.  For directories check if we are also going to generate
+	 default entries.  If not we have a problem.  We can't generate only a
+	 single, inheritable NULL SID ACE because that leads to (fixable, TODO)
+	 access problems when trying to create the matching child permissions.
+	 Therefore we remove the S_ISGID bit on the directory because having it
+	 set would be misleading. */
+      if (!def && S_ISDIR (attr) && (attr & S_ISGID))
+	{
+	  /* Check for a required entry per POSIX. */
+	  tmp_idx = searchace (aclbufp, nentries, DEF_USER_OBJ);
+	  if (tmp_idx < 0)
+	    attr &= ~S_ISGID;
+	}
       access = CYG_ACE_ISBITS_TO_WIN (def ? attr & S_ISGID : attr)
 	       | CYG_ACE_NEW_STYLE;
       tmp_idx = searchace (aclbufp, nentries, def | CLASS_OBJ);

More information about the Cygwin-cvs mailing list