From 52f882dba207305c3fadf7b702cf1f36702865e9 Mon Sep 17 00:00:00 2001 From: Trung Date: Mon, 3 Aug 2026 22:09:04 +0700 Subject: [PATCH] Ignore setuid from owners the guest cannot mean execve took the new effective ID straight from a raw host fstat. A sysroot is an ordinary tree owned by whoever unpacked it and no host IDs are mapped into the guest, so a setuid binary left the process at an ID that exists nowhere in the guest -- neither root nor its own. Privilege checks against euid 0 failed, ownership comparisons against guest IDs failed, and the ID granted followed whoever owned the tree. Judge access through chown_overlay_apply, the way fs-stat.c reports ownership, so exec and stat cannot disagree about who owns a file. Trusting the overlay there is a trade rather than an oversight: any guest can write it, but the host open still gates the physical file, so the most it buys is a check the physical mode would have refused. The set-id owner takes both views and grants only where they agree. Reading it through the overlay alone would make root self-service, since an overlay owner of 0 proves nothing about privilege: chown your own file to 0, set the bit, exec. Reading it from the host alone would elevate on ownership the guest's own stat denies, since a physically root-owned file kept elevating after a guest chowned it away. Agreement grants nothing new -- root still means a file the host really owns as root -- and lets a guest chown withdraw an elevation but never conjure one. Honour the bit only for root or the caller's own ID and leave the ID untouched otherwise, matching what Linux does for a setuid binary on a nosuid mount rather than failing the exec. This cuts both ways: a caller already at euid 0 that execs a setuid file owned by a host ID now keeps root instead of dropping to an ID the guest cannot mean. Widening the rule for privileged callers would reintroduce the unmapped-host-ID outcome this fixes. tests/test-setuid-exec.c pins both directions. Unprivileged it stages a foreign owner and a recorded root owner and checks neither moves the effective ID, and that an owner-only file the guest owns still runs. Under --fakeroot it checks root is not pulled down to the host owner. Against the previous exec.c the unprivileged lane fails all three checks and the fakeroot lane fails its one, and a run refuses to report success when no check exercised an exec. Both lanes reason about elfuse-only state, so the qemu lane skips them. Addresses #264 --- src/syscall/exec.c | 86 +++++++++-- tests/test-matrix.sh | 12 +- tests/test-setuid-exec.c | 306 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 394 insertions(+), 10 deletions(-) create mode 100644 tests/test-setuid-exec.c diff --git a/src/syscall/exec.c b/src/syscall/exec.c index 8f834599..fa3d40c1 100644 --- a/src/syscall/exec.c +++ b/src/syscall/exec.c @@ -35,6 +35,7 @@ #include "runtime/futex.h" #include "syscall/abi.h" +#include "syscall/chown-overlay.h" #include "syscall/exec.h" #include "syscall/fuse.h" #include "syscall/internal.h" @@ -369,6 +370,45 @@ static bool exec_matches_fakeroot_target(const struct stat *st) return marked_st.st_dev == st->st_dev && marked_st.st_ino == st->st_ino; } +/* Is ID something the guest can actually mean? + * + * A sysroot is an ordinary directory tree owned by whoever unpacked it, and + * elfuse maps no host IDs into the guest, so most files report the invoking + * macOS user (e.g. 501) -- an ID that exists nowhere in the guest's own + * /etc/passwd. Honouring setuid on such a file would leave the process at an + * effective ID that is neither root nor its own, failing both `euid == 0` + * privilege checks and ownership comparisons against guest IDs, and the + * granted ID would follow whoever happens to own the tree rather than anything + * the guest chose. + * + * Accept only root and the caller's own ID, and only when both views of + * ownership agree on it. + * + * physical is what the host reports; seen is what the guest's own stat reports, + * after the virtual chown overlay. Elevating needs both because each view alone + * fails in a different direction. Reading the set-id owner through the overlay + * would make root self-service, since the overlay takes any unprivileged + * guest's chown -- chown its own file to 0, set the bit, exec. Reading it only + * from the host would let a file the guest's own stat says belongs to someone + * else still elevate, because a physically root-owned file keeps elevating + * after the guest chowns it away. + * + * Requiring agreement grants nothing new: root still means a file the host + * really owns as root, and a guest chown can only ever withdraw an elevation, + * never create one. The caller's own ID makes the bit a no-op, as on Linux. + * Anything else leaves the ID untouched and the program simply runs + * unprivileged -- the outcome Linux gives for a setuid binary on a nosuid + * mount, and never an exec failure. + */ +static bool exec_id_may_elevate(uint32_t physical, + uint32_t seen, + uint32_t self_id) +{ + if (physical != seen) + return false; + return physical == 0 || physical == self_id; +} + static int check_exec_permission(const struct stat *st) { uint32_t uid = proc_get_euid(); @@ -549,8 +589,22 @@ int64_t sys_execve(hv_vcpu_t vcpu, err = linux_errno(); goto fail; } + /* Judge access by the ownership the guest sees: fs-stat.c reports every + * guest stat through the virtual chown overlay, so the physical owner would + * refuse a file the guest's own stat says it may execute. Trusting a + * guest-writable overlay here -- chown_result records the intended owner on + * any EPERM, unchecked -- is a deliberate trade. A guest can chown a file + * into its own name to pass a check the physical mode refuses, and gains + * nothing: the host open must still succeed on the physical file. + * + * exec_st itself stays physical, and the set-id decision below takes both: + * it grants only where the two agree, so the overlay can withdraw an + * elevation but never conjure one. + */ + struct stat exec_st_seen = exec_st; + chown_overlay_apply(&exec_st_seen); - err = check_exec_permission(&exec_st); + err = check_exec_permission(&exec_st_seen); if (err < 0) { goto fail; } @@ -710,6 +764,7 @@ int64_t sys_execve(hv_vcpu_t vcpu, err = linux_errno(); goto fail; } + chown_overlay_apply(&interp_st); err = check_exec_permission(&interp_st); if (err < 0) { goto fail; @@ -721,20 +776,32 @@ int64_t sys_execve(hv_vcpu_t vcpu, goto fail; } - /* Compute setuid/setgid from the directly-executed file, matching Linux - * kernel behaviour (fs/exec.c bprm_fill_uid). Scripts are - * deliberately excluded: the kernel ignores setuid/setgid on shebang - * scripts to prevent privilege escalation via interpreter manipulation. - * S_ISGID is only effective when the group-execute bit is also set, - * matching the kernel's mandatory-locking vs setgid distinction. + /* Compute setuid/setgid from the directly-executed file (fs/exec.c + * bprm_fill_uid). A set-id bit on the shebang script itself is ignored, as + * on Linux, so the interpreter named in the script cannot be manipulated + * into carrying the script's privilege. S_ISGID is only effective when the + * group-execute bit is also set, matching the kernel's mandatory-locking vs + * setgid distinction. + * + * Where this deliberately stops short of Linux: the kernel derives file + * creds from the file it finally executes, so a set-id *interpreter* named + * in a "#!" line does elevate there. exec_st is the directly-executed file + * and is never refreshed after the shebang loop, so it does not here. + * Elevating through an interpreter is unsupported, not accidentally lost. */ uint32_t new_euid = proc_get_euid(); uint32_t new_egid = proc_get_egid(); if (have_exec_st && !exec_is_script && S_ISREG(exec_st.st_mode)) { - if (exec_st.st_mode & S_ISUID) { + if ((exec_st.st_mode & S_ISUID) && + exec_id_may_elevate((uint32_t) exec_st.st_uid, + (uint32_t) exec_st_seen.st_uid, + proc_get_uid())) { new_euid = (uint32_t) exec_st.st_uid; } - if ((exec_st.st_mode & S_ISGID) && (exec_st.st_mode & S_IXGRP)) { + if ((exec_st.st_mode & S_ISGID) && (exec_st.st_mode & S_IXGRP) && + exec_id_may_elevate((uint32_t) exec_st.st_gid, + (uint32_t) exec_st_seen.st_gid, + proc_get_gid())) { new_egid = (uint32_t) exec_st.st_gid; } } @@ -844,6 +911,7 @@ int64_t sys_execve(hv_vcpu_t vcpu, err = linux_errno(); goto fail; } + chown_overlay_apply(&interp_st); err = check_exec_permission(&interp_st); if (err < 0) { diff --git a/tests/test-matrix.sh b/tests/test-matrix.sh index 7fc9c10d..592a6d99 100755 --- a/tests/test-matrix.sh +++ b/tests/test-matrix.sh @@ -277,6 +277,8 @@ QEMU_SKIP=" test-credentials test-credentials-fakeroot test-fakeroot-exec + test-setuid-exec + test-setuid-exec-fakeroot test-sched-policy test-rseq test-tier-a @@ -331,6 +333,10 @@ QEMU_SKIP=" # test-fakeroot-exec: ELFUSE_FAKEROOT_EXEC is an elfuse-only escape hatch -- # a real kernel has no notion of an executable that turns on fakeroot, so # the marked exec simply stays unprivileged there. +# test-setuid-exec / -fakeroot: reasons about elfuse's virtual chown overlay +# and about host UIDs no sysroot maps into the guest, neither of which a +# real kernel has; the qemu reference lane also runs as genuine root, where +# a chown to any owner succeeds physically and setuid is honoured from it. # test-sched-policy: exercises elfuse's explicitly-a-stub scheduler policy # layer (see the file's own header) -- RT class changes are always # -EPERM'd regardless of privilege, whereas real root can set them. @@ -775,6 +781,10 @@ run_unit_tests() # interrupted run cannot leave it set for anything that follows. ELFUSE_FAKEROOT_EXEC="$bindir/test-fakeroot-exec" \ test_rc "$runner" "test-fakeroot-exec" 0 "$bindir/test-fakeroot-exec" + test_check "$runner" "test-setuid-exec" "all tests passed" \ + "$bindir/test-setuid-exec" + test_check "$runner" "test-setuid-exec-fakeroot" "all tests passed" \ + --fakeroot "$bindir/test-setuid-exec" printf "\nScheduler policy stub\n" test_rc "$runner" "test-sched-policy" 0 "$bindir/test-sched-policy" @@ -1242,7 +1252,7 @@ run_suite() # observed counts diverge. apple-unknown is the fallback row for SoC strings the # detector does not recognize yet. EXPECTED_BASELINES=( - "elfuse-aarch64|239|0" + "elfuse-aarch64|241|0" "qemu-aarch64|218|0" "elfuse-x86_64:apple-m1-m2|71|0" "elfuse-x86_64:apple-m3-plus|71|0" diff --git a/tests/test-setuid-exec.c b/tests/test-setuid-exec.c new file mode 100644 index 00000000..327f706f --- /dev/null +++ b/tests/test-setuid-exec.c @@ -0,0 +1,306 @@ +/* setuid-on-exec must grant an ID the guest can actually mean. + * + * A sysroot's files are owned by whoever unpacked them on the host, and no host + * IDs are mapped into the guest. Honouring setuid on such a file would leave + * the process at an effective ID that exists nowhere in the guest -- neither + * root nor its own -- so the bit has to be ignored there, the way Linux ignores + * it on a nosuid mount. + * + * Ownership the guest merely recorded is no better. The virtual chown overlay + * accepts any unprivileged guest's chown, so an owner of 0 that came from it + * proves nothing about privilege; only physical root ownership may elevate. + * That is the case this test cannot stage without being root already, so what + * it pins is the other half: a virtual owner, foreign or root, never elevates. + * + * Note which owner does the work. The set-id path reads the *physical* stat, + * and mkstemp already leaves the helper owned by the host user (501 or + * whatever unpacked the tree), which is foreign to the guest either way. The + * virtual chowns below stage the guest-visible half; the physical host owner + * is what the exec actually sees and refuses to honour. + * + * Copyright 2026 elfuse contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +/* An ID that is neither root nor any caller this test runs as, so the "owner + * the guest cannot mean" case is staged deterministically instead of relying on + * whichever host UID happens to own /tmp files. + */ +#define FOREIGN_UID 4242u + +/* Unique per invocation. A fixed name in the shared /tmp would let two runs + * truncate each other's helper, and the cleanup unlink would remove a file a + * concurrent run still needs. mkstemp creates it O_EXCL and mode 0600, so no + * other run can be holding the same path. + */ +static char helper_path[] = "/tmp/test-setuid-exec-XXXXXX"; + +/* Re-executed as the setuid helper: report whether euid matches expectations. + */ +static int child_main(const char *want_str) +{ + unsigned long want = strtoul(want_str, NULL, 10); + unsigned got = (unsigned) geteuid(); + if ((unsigned long) got != want) { + fprintf(stderr, " child: euid=%u expected=%lu\n", got, want); + return 2; + } + return 0; +} + +/* Copy this binary to a fresh helper_path and mark it setuid. The fd is closed + * before returning: exec'ing a file still open for writing is ETXTBSY. + */ +static int build_setuid_helper(void) +{ + char src[4096]; + ssize_t n = readlink("/proc/self/exe", src, sizeof(src) - 1); + if (n <= 0) + return -1; + src[n] = '\0'; + + int in = open(src, O_RDONLY); + if (in < 0) + return -1; + int out = mkstemp(helper_path); + if (out < 0) { + close(in); + return -1; + } + + /* Past mkstemp the file exists, so every failure below has to unlink it + * rather than leave a setuid-marked copy of this binary in the shared /tmp. + */ + int rc = 0; + char buf[65536]; + ssize_t got = 0; + while ((got = read(in, buf, sizeof(buf))) > 0) { + ssize_t off = 0; + while (off < got) { + ssize_t put = write(out, buf + off, (size_t) (got - off)); + if (put <= 0) { + rc = -1; + break; + } + off += put; + } + if (rc != 0) + break; + } + close(in); + /* Set the bit through the fd we own rather than re-opening by name. */ + if (rc == 0 && (got < 0 || fchmod(out, 04755) != 0)) + rc = -1; + if (close(out) != 0 && rc == 0) + rc = -1; + if (rc != 0) + (void) unlink(helper_path); + return rc; +} + +/* Run the helper and return its exit status, or -1 if it could not run. */ +static int run_helper(unsigned want_euid) +{ + char expect[32]; + snprintf(expect, sizeof(expect), "%u", want_euid); + + pid_t pid = fork(); + if (pid < 0) + return -1; + if (pid == 0) { + execl(helper_path, helper_path, "--child", expect, (char *) NULL); + _exit(127); + } + int status = 0; + if (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status)) + return -1; + return WEXITSTATUS(status); +} + +/* Hand the helper to owner and re-arm the setuid bit, which a chown drops. + * Returns 1 when the guest-visible owner really is owner afterwards, 0 when the + * chown was refused, -1 when the staging itself broke. + */ +static int stage_owner(unsigned owner) +{ + errno = 0; + if (chown(helper_path, (uid_t) owner, (gid_t) -1) != 0) + return 0; + if (chmod(helper_path, 04755) != 0) + return -1; + struct stat st; + if (stat(helper_path, &st) != 0) + return -1; + /* exec reads ownership the same way stat does, so a stat that does not show + * the chown means the two would disagree about what was staged. + */ + if ((unsigned) st.st_uid != owner) { + fprintf(stderr, " stat reports owner=%u after chown to %u\n", + (unsigned) st.st_uid, owner); + return -1; + } + return 1; +} + +/* Whether the *guest* runs as root, which decides what a chown means to the + * emulation. It says nothing about the host: elfuse can be running as host root + * while the guest is uid 1000, so a chown the guest performs may still land + * physically. Checks that turn on that difference resolve it from the observed + * outcome rather than from this. + */ +static int caller_is_privileged(void) +{ + return geteuid() == 0; +} + +int main(int argc, char **argv) +{ + if (argc >= 3 && strcmp(argv[1], "--child") == 0) + return child_main(argv[2]); + + int failures = 0; + /* Checks that actually exercised an exec. Without this a run where every + * interesting case skipped -- a privileged caller, say -- would print the + * success line and exit 0 having proved nothing. + */ + int checks_run = 0; + unsigned self_uid = (unsigned) getuid(); + unsigned self_euid = (unsigned) geteuid(); + + printf("test-setuid-exec: 1. build setuid helper... "); + if (build_setuid_helper() != 0) { + printf("FAIL (could not build helper: %m)\n"); + return 1; + } + struct stat st; + if (stat(helper_path, &st) != 0) { + printf("FAIL (stat: %m)\n"); + failures++; + goto cleanup; + } + printf("PASS (owner=%u mode=%04o)\n", (unsigned) st.st_uid, + st.st_mode & 07777); + + /* 2. An owner the guest cannot mean must not become the effective ID. + * + * No skip: the physical owner staged by mkstemp is already foreign to the + * guest, whichever host user that is, so the case holds whether or not the + * virtual chown below is recorded. Attempting the chown on top only widens + * it -- the guest then sees a foreign owner too, and both views agree. + */ + printf("test-setuid-exec: 2. foreign owner does not elevate... "); + { + unsigned physical_owner = (unsigned) st.st_uid; + int staged = caller_is_privileged() ? 0 : stage_owner(FOREIGN_UID); + if (staged < 0) { + printf("FAIL (could not stage foreign owner)\n"); + failures++; + } else { + checks_run++; + int rc = run_helper(self_euid); + if (rc < 0 || rc == 127) { + printf("FAIL (helper did not run)\n"); + failures++; + } else if (rc != 0) { + printf("FAIL (euid moved to an owner the guest cannot mean)\n"); + failures++; + } else if (staged == 1) { + printf("PASS (owner=%u physical, %u virtual, euid stayed %u)\n", + physical_owner, FOREIGN_UID, self_euid); + } else { + printf("PASS (physical owner=%u ignored, euid stayed %u)\n", + physical_owner, self_euid); + } + } + } + + /* 3. Nor may a *recorded* root owner. The chown overlay takes any guest's + * chown, so honouring the owner it reports would let an unprivileged + * process chown its own file to 0, set the bit, and exec its way to root. + * The regression guard for that escalation. + * + * A guest stat cannot tell a recorded chown from one the host really + * performed -- the overlay is invisible from in here, and both report + * owner 0. That distinction matters, because when elfuse itself runs as + * host root the chown below is physical, and a physically root-owned + * setuid file is *supposed* to elevate. Resolve it from the outcome + * instead: elevation to 0 only happens when the physical owner is 0, which + * only a real chown can produce. Staying put and landing on root are both + * correct, one per staging; anything else is the bug. + */ + printf("test-setuid-exec: 3. virtual root owner does not elevate... "); + { + int staged = stage_owner(0); + if (staged < 0) { + printf("FAIL (could not stage root owner)\n"); + failures++; + } else if (staged == 0) { + printf("SKIP (chown to root refused, as on a real kernel)\n"); + } else { + checks_run++; + int rc = run_helper(self_euid); + if (rc < 0 || rc == 127) { + printf("FAIL (helper did not run)\n"); + failures++; + } else if (rc == 0) { + printf("PASS (stat says owner=0, euid stayed %u)\n", self_euid); + } else if (run_helper(0) == 0) { + printf("PASS (host chown was real, physical root elevated)\n"); + } else { + printf("FAIL (euid is neither %u nor 0; see the report)\n", + self_euid); + failures++; + } + } + } + + /* 4. The permission check must use the owner the guest sees, unlike the + * set-id owner above. A file the guest owns with owner-only permission has + * to run: judging it by the physical owner would fall through to the + * "other" bits and refuse an executable the guest's own stat says it owns. + */ + printf("test-setuid-exec: 4. guest-owned owner-only file runs... "); + if (chown(helper_path, self_uid, (gid_t) -1) != 0) { + printf("SKIP (cannot take ownership: %m)\n"); + } else if (chmod(helper_path, 0700) != 0) { + printf("FAIL (chmod: %m)\n"); + failures++; + } else { + checks_run++; + int rc = run_helper(self_euid); + if (rc < 0) { + printf("FAIL (helper did not run at all)\n"); + failures++; + } else if (rc == 127) { + printf("FAIL (exec refused for a file the guest owns)\n"); + failures++; + } else if (rc != 0) { + printf("FAIL (helper exited %d)\n", rc); + failures++; + } else { + printf("PASS\n"); + } + } + +cleanup: + (void) unlink(helper_path); + + if (failures == 0 && checks_run == 0) { + printf("test-setuid-exec: no check exercised an exec -- FAIL\n"); + return 1; + } + if (failures == 0) + printf("test-setuid-exec: all tests passed -- PASS\n"); + else + printf("test-setuid-exec: %d failed\n", failures); + return failures != 0; +}