Skip to content

Commit a7881d7

Browse files
committed
test: check supplementary groups and ACLs for -r/-w/-x permissions.
Replace manual permission checking logic with platform-specific libc functions that properly handle supplementary groups and ACLs: - Linux, Hurd, Redox, Cygwin, Solaris, Illumos: euidaccess() - FreeBSD: eaccess() - macOS, iOS, NetBSD, OpenBSD, DragonFly: faccessat() with AT_EACCESS - Android: faccessat() - Other OSes: fall back to libc:access() These functions delegate permission checks to the kernel, which correctly evaluates all of the user's group memberships (both primary and supplementary), as well as ACLs and other advanced permission features. Fixes #8960 Fixes #9147
1 parent 3220258 commit a7881d7

File tree

3 files changed

+99
-27
lines changed

3 files changed

+99
-27
lines changed

.vscode/cspell.dictionaries/workspace.wordlist.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,9 @@ vmsplice
152152

153153
# * vars/libc
154154
COMFOLLOW
155+
EACCESS
155156
EXDEV
157+
FDCWD
156158
FILENO
157159
FTSENT
158160
HOSTSIZE
@@ -196,7 +198,10 @@ blocksize
196198
canonname
197199
chroot
198200
dlsym
201+
eaccess
202+
euidaccess
199203
execvp
204+
faccessat
200205
fdatasync
201206
freeaddrinfo
202207
getaddrinfo

src/uu/test/src/test.rs

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -258,26 +258,71 @@ enum PathCondition {
258258

259259
#[cfg(not(windows))]
260260
fn path(path: &OsStr, condition: &PathCondition) -> bool {
261-
use std::fs::Metadata;
261+
use std::ffi::CString;
262+
use std::os::unix::ffi::OsStrExt;
262263
use std::os::unix::fs::FileTypeExt;
263264

264265
const S_ISUID: u32 = 0o4000;
265266
const S_ISGID: u32 = 0o2000;
266267
const S_ISVTX: u32 = 0o1000;
267268

268-
enum Permission {
269-
Read = 0o4,
270-
Write = 0o2,
271-
Execute = 0o1,
272-
}
269+
// Helper function to check file access permissions.
270+
// Uses platform-specific access functions that properly handle supplementary groups
271+
// and ACLs.
272+
let check_access = |path: &OsStr, mode: i32| -> bool {
273+
let path_bytes = path.as_bytes();
274+
let Ok(c_path) = CString::new(path_bytes) else {
275+
return false; // Path contains null byte
276+
};
273277

274-
let perm = |metadata: Metadata, p: Permission| {
275-
if geteuid() == metadata.uid() {
276-
metadata.mode() & ((p as u32) << 6) != 0
277-
} else if getegid() == metadata.gid() {
278-
metadata.mode() & ((p as u32) << 3) != 0
279-
} else {
280-
metadata.mode() & (p as u32) != 0
278+
unsafe {
279+
#[cfg(any(
280+
target_os = "linux",
281+
target_os = "hurd",
282+
target_os = "redox",
283+
target_os = "cygwin",
284+
target_os = "solaris",
285+
target_os = "illumos"
286+
))]
287+
{
288+
libc::euidaccess(c_path.as_ptr(), mode) == 0
289+
}
290+
291+
#[cfg(target_os = "freebsd")]
292+
{
293+
libc::eaccess(c_path.as_ptr(), mode) == 0
294+
}
295+
296+
#[cfg(any(
297+
target_os = "macos",
298+
target_os = "ios",
299+
target_os = "netbsd",
300+
target_os = "openbsd",
301+
target_os = "dragonfly"
302+
))]
303+
{
304+
libc::faccessat(libc::AT_FDCWD, c_path.as_ptr(), mode, libc::AT_EACCESS) == 0
305+
}
306+
307+
// Fallback for other OSes
308+
#[cfg(not(any(
309+
target_os = "linux",
310+
target_os = "hurd",
311+
target_os = "redox",
312+
target_os = "cygwin",
313+
target_os = "solaris",
314+
target_os = "illumos",
315+
target_os = "freebsd",
316+
target_os = "macos",
317+
target_os = "ios",
318+
target_os = "netbsd",
319+
target_os = "openbsd",
320+
target_os = "dragonfly"
321+
)))]
322+
{
323+
// fallback: use regular access()
324+
libc::access(c_path.as_ptr(), mode) == 0
325+
}
281326
}
282327
};
283328

@@ -308,12 +353,12 @@ fn path(path: &OsStr, condition: &PathCondition) -> bool {
308353
PathCondition::Sticky => metadata.mode() & S_ISVTX != 0,
309354
PathCondition::UserOwns => metadata.uid() == geteuid(),
310355
PathCondition::Fifo => file_type.is_fifo(),
311-
PathCondition::Readable => perm(metadata, Permission::Read),
356+
PathCondition::Readable => check_access(path, libc::R_OK),
312357
PathCondition::Socket => file_type.is_socket(),
313358
PathCondition::NonEmpty => metadata.size() > 0,
314359
PathCondition::UserIdFlag => metadata.mode() & S_ISUID != 0,
315-
PathCondition::Writable => perm(metadata, Permission::Write),
316-
PathCondition::Executable => perm(metadata, Permission::Execute),
360+
PathCondition::Writable => check_access(path, libc::W_OK),
361+
PathCondition::Executable => check_access(path, libc::X_OK),
317362
}
318363
}
319364

tests/by-util/test_test.rs

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,8 +1028,22 @@ fn test_string_lt_gt_operator() {
10281028
.no_output();
10291029
}
10301030

1031-
#[test]
1032-
#[cfg(unix)]
1031+
// Only test platforms supporting euidaccess(), eaccess() or faccessat() in src/uu/test/src/test.rs::path
1032+
#[test]
1033+
#[cfg(any(
1034+
target_os = "linux",
1035+
target_os = "hurd",
1036+
target_os = "redox",
1037+
target_os = "cygwin",
1038+
target_os = "solaris",
1039+
target_os = "illumos",
1040+
target_os = "freebsd",
1041+
target_os = "macos",
1042+
target_os = "ios",
1043+
target_os = "netbsd",
1044+
target_os = "openbsd",
1045+
target_os = "dragonfly"
1046+
))]
10331047
fn test_permission_with_supplementary_group() {
10341048
// Test that permission checks (-r, -w, -x) correctly consider supplementary groups,
10351049
// not just the primary group (egid). See #9147
@@ -1048,13 +1062,12 @@ fn test_permission_with_supplementary_group() {
10481062
}
10491063

10501064
// Get user's supplementary groups
1051-
let groups_output = match Command::new("id").arg("-nG").output() {
1052-
Ok(output) => output,
1053-
Err(_) => return,
1065+
let Ok(groups_output) = Command::new("id").arg("-nG").output() else {
1066+
return;
10541067
};
10551068

10561069
let groups_str = String::from_utf8_lossy(&groups_output.stdout);
1057-
let groups: Vec<&str> = groups_str.trim().split_whitespace().collect();
1070+
let groups: Vec<&str> = groups_str.split_whitespace().collect();
10581071

10591072
// Need at least 2 groups (primary + at least one supplementary)
10601073
if groups.len() < 2 {
@@ -1104,7 +1117,17 @@ fn test_permission_with_supplementary_group() {
11041117
}
11051118

11061119
#[test]
1107-
#[cfg(unix)]
1120+
#[cfg(any(
1121+
target_os = "linux",
1122+
target_os = "redox",
1123+
target_os = "cygwin",
1124+
target_os = "solaris",
1125+
target_os = "illumos",
1126+
target_os = "freebsd",
1127+
target_os = "netbsd",
1128+
target_os = "openbsd",
1129+
target_os = "dragonfly"
1130+
))]
11081131
fn test_permission_with_acl() {
11091132
// Test that permission checks (-r, -w, -x) correctly handle ACL (Access Control List)
11101133
// permissions, not just traditional Unix permissions. This requires using euidaccess()
@@ -1129,9 +1152,8 @@ fn test_permission_with_acl() {
11291152
}
11301153

11311154
// Get current username for ACL
1132-
let username_output = match Command::new("whoami").output() {
1133-
Ok(output) => output,
1134-
Err(_) => return,
1155+
let Ok(username_output) = Command::new("whoami").output() else {
1156+
return;
11351157
};
11361158
let username = String::from_utf8_lossy(&username_output.stdout)
11371159
.trim()
@@ -1170,7 +1192,7 @@ fn test_permission_with_acl() {
11701192
"--non-interactive",
11711193
"setfacl",
11721194
"-m",
1173-
&format!("u:{}:rx", username),
1195+
&format!("u:{username}:rx"),
11741196
test_path_str,
11751197
])
11761198
.run();

0 commit comments

Comments
 (0)