diff options
| author | Corinna Vinschen <corinna@vinschen.de> | 2015-02-27 15:59:09 +0300 |
|---|---|---|
| committer | Corinna Vinschen <corinna@vinschen.de> | 2015-02-27 15:59:09 +0300 |
| commit | 06371539bd405ce4ba288d44efaaeb645ed37299 (patch) | |
| tree | 73d26a8ee4241d6d184245a173b89f2f62fa32c7 /winsup/cygwin/security.cc | |
| parent | d2f9dbb3ee0fc1a8129aac8a42f1ce77e43ac4e9 (diff) | |
* 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".
Diffstat (limited to 'winsup/cygwin/security.cc')
| -rw-r--r-- | winsup/cygwin/security.cc | 23 |
1 files changed, 20 insertions, 3 deletions
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; } |