Skip to content

Commit 7ff63fb

Browse files
committed
win,fs: Do not allow setting troublesome permissions on Windows
Because our ACL implementation does not have the required flexibility to represent situations such as `0o757` due to the permissions ordering that Windows naturally applies, we simply do not allow the `other` entity to have permissions that the `group` entity does not have. This causes `chmod(0o757)` to be transparently mapped to `chmod(0o755)`, as an example.
1 parent 7d69a03 commit 7ff63fb

File tree

1 file changed

+23
-13
lines changed

1 file changed

+23
-13
lines changed

src/win/fs.c

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2417,7 +2417,8 @@ static void fs__chmod(uv_fs_t* req) {
24172417
/* Skip this EA if it isn't an SID, or it is "Everyone" or our actual group */
24182418
if (pOldEAs[ea_idx].Trustee.TrusteeForm != TRUSTEE_IS_SID ||
24192419
EqualSid(pEASid, psidEveryone) ||
2420-
EqualSid(pEASid, psidGroup)) {
2420+
EqualSid(pEASid, psidGroup) ||
2421+
EqualSid(pEASid, psidOwner)) {
24212422
continue;
24222423
}
24232424

@@ -2434,43 +2435,52 @@ static void fs__chmod(uv_fs_t* req) {
24342435
}
24352436

24362437
/* Create an ACE for each triplet (user, group, other) */
2437-
numNewEAs = 8 + 3*numOtherGroups;
2438+
numNewEAs = 7 + 3*numOtherGroups;
24382439
ea = (PEXPLICIT_ACCESS_W) malloc(sizeof(EXPLICIT_ACCESS_W)*numNewEAs);
24392440
u_mode = ((req->fs.info.mode & S_IRWXU) >> 6);
24402441
g_mode = ((req->fs.info.mode & S_IRWXG) >> 3);
2441-
o_mode = ((req->fs.info.mode & S_IRWXO) >> 0);
2442+
2443+
/*
2444+
* Because we do not control the ordering of ACE entries within the ACL that
2445+
* we're building, the `SetNamedSecurityInfoW()` function call below will
2446+
* place all DENY entries first, and all `ALLOW` entries second. This
2447+
* makes it impossible to support e.g. 0o757 permissions, because in order
2448+
* to support an "allow" for "other", then a "deny" for "group", we are
2449+
* unable to have an "allow" before the "deny" for "group". To address this,
2450+
* we simply do not allow the "other" entity to have permissions that "group"
2451+
* itself does not have.
2452+
*/
2453+
o_mode = ((req->fs.info.mode & S_IRWXO) >> 0) & g_mode;
24422454

24432455
/* We start by revoking previous permissions for trustees we care about */
24442456
build_access_struct(&ea[0], psidOwner, TRUSTEE_IS_USER, 0, REVOKE_ACCESS);
24452457
build_access_struct(&ea[1], psidGroup, TRUSTEE_IS_GROUP, 0, REVOKE_ACCESS);
24462458
build_access_struct(&ea[2], psidEveryone, TRUSTEE_IS_GROUP, 0, REVOKE_ACCESS);
24472459

24482460
/*
2449-
* We also add explicit denies to user and group if the user shouldn't have
2450-
* a permission but the group or everyone can, for instance.
2461+
* We also add explicit denies to user if the group shouldn't have a permission.
24512462
*/
2452-
u_deny_mode = (~u_mode) & (g_mode | o_mode);
2453-
g_deny_mode = (~g_mode) & o_mode;
2463+
u_deny_mode = (~u_mode) & (g_mode);
24542464
build_access_struct(&ea[3], psidOwner, TRUSTEE_IS_USER, u_deny_mode, DENY_ACCESS);
2455-
build_access_struct(&ea[4], psidGroup, TRUSTEE_IS_GROUP, g_deny_mode, DENY_ACCESS);
24562465

24572466
/* Next, add explicit allows for (owner, group, other) */
2458-
build_access_struct(&ea[5], psidOwner, TRUSTEE_IS_USER, u_mode, SET_ACCESS);
2459-
build_access_struct(&ea[6], psidGroup, TRUSTEE_IS_GROUP, g_mode, SET_ACCESS);
2460-
build_access_struct(&ea[7], psidEveryone, TRUSTEE_IS_GROUP, o_mode, SET_ACCESS);
2467+
build_access_struct(&ea[4], psidOwner, TRUSTEE_IS_USER, u_mode, SET_ACCESS);
2468+
build_access_struct(&ea[5], psidGroup, TRUSTEE_IS_GROUP, g_mode, SET_ACCESS);
2469+
build_access_struct(&ea[6], psidEveryone, TRUSTEE_IS_GROUP, o_mode, SET_ACCESS);
24612470

24622471
/*
24632472
* Iterate over all old ACEs, looking for groups that we belong to, and setting
24642473
* the appropriate access bits for those groups (as g_mode)
24652474
*/
2466-
ea_write_idx = 8;
2475+
ea_write_idx = 7;
24672476
for (ea_idx=0; ea_idx<numOldEAs; ++ea_idx) {
24682477
BOOL isMember = FALSE;
24692478
PSID pEASid = (PSID)pOldEAs[ea_idx].Trustee.ptstrName;
24702479
/* Skip this EA if it isn't an SID, or it is "Everyone" or our actual group */
24712480
if (pOldEAs[ea_idx].Trustee.TrusteeForm != TRUSTEE_IS_SID ||
24722481
EqualSid(pEASid, psidEveryone) ||
2473-
EqualSid(pEASid, psidGroup)) {
2482+
EqualSid(pEASid, psidGroup) ||
2483+
EqualSid(pEASid, psidOwner)) {
24742484
continue;
24752485
}
24762486

0 commit comments

Comments
 (0)