From 06371539bd405ce4ba288d44efaaeb645ed37299 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Fri, 27 Feb 2015 12:59:09 +0000 Subject: * sec_acl.cc (setacl): Fix bug which leads to ACE duplication in case owner SID == group SID. (getacl): Reverse order of SID test against group or owner sid to prefer owner attributes over group attributes. Disable setting group permissions equivalent to owner permissions if owner == group. Add comment to explain why. Fix indentation. * security.cc (get_attribute_from_acl): Change type of local variables containing permission to mode_t. Apply deny mask to group if group SID == owner SID to avoid Everyone permissions to spill over into group permissions. Disable setting group permissions equivalent to owner permissions if owner == group. Add comment to explain why. * uinfo.cc (pwdgrp::fetch_account_from_windows): Allow user SID as group account if user is a "Microsoft Account". Explain why. Drop workaround enforcing primary group "Users" for "Microsoft Accounts". --- winsup/cygwin/security.cc | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'winsup/cygwin/security.cc') diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index 9c94c7053..6dde7d3c8 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -239,9 +239,9 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid, PSID group_sid, bool grp_member) { ACCESS_ALLOWED_ACE *ace; - int allow = 0; - int deny = 0; - int *flags, *anti; + mode_t allow = 0; + mode_t deny = 0; + mode_t *flags, *anti; for (DWORD i = 0; i < acl->AceCount; ++i) { @@ -301,6 +301,17 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid, *flags |= ((!(*anti & S_IWUSR)) ? S_IWUSR : 0); if (ace->Mask & FILE_EXEC_BITS) *flags |= ((!(*anti & S_IXUSR)) ? S_IXUSR : 0); + /* Apply deny mask to group if group SID == owner SID. */ + if (group_sid && RtlEqualSid (owner_sid, group_sid) + && ace->Header.AceType == ACCESS_DENIED_ACE_TYPE) + { + if (ace->Mask & FILE_READ_BITS) + *flags |= ((!(*anti & S_IRUSR)) ? S_IRGRP : 0); + if (ace->Mask & FILE_WRITE_BITS) + *flags |= ((!(*anti & S_IWUSR)) ? S_IWGRP : 0); + if (ace->Mask & FILE_EXEC_BITS) + *flags |= ((!(*anti & S_IXUSR)) ? S_IXGRP : 0); + } } else if (ace_sid == group_sid) { @@ -331,6 +342,11 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid, } } *attribute &= ~(S_IRWXU | S_IRWXG | S_IRWXO | S_ISVTX | S_ISGID | S_ISUID); +#if 0 + /* Disable owner/group permissions equivalence if owner SID == group SID. + It's technically not quite correct, but it helps in case a security + conscious application checks if a file has too open permissions. In + fact, since owner == group, there's no security issue here. */ if (owner_sid && group_sid && RtlEqualSid (owner_sid, group_sid) /* FIXME: temporary exception for /var/empty */ && well_known_system_sid != group_sid) @@ -340,6 +356,7 @@ get_attribute_from_acl (mode_t *attribute, PACL acl, PSID owner_sid, | ((allow & S_IWUSR) ? S_IWGRP : 0) | ((allow & S_IXUSR) ? S_IXGRP : 0)); } +#endif *attribute |= allow; } -- cgit v1.2.3