Suppress setuid and setgid on shebang scripts#199
Conversation
The Linux kernel (fs/exec.c bprm_fill_uid) deliberately ignores setuid/setgid bits on interpreted scripts to prevent privilege escalation via interpreter manipulation. elfuse had no such guard: exec_is_script was never tracked, so the setuid/setgid check was effectively always true for all executables. Fix by: - Snapshotting the directly-executed file's stat() before the shebang resolution loop overwrites path_host with the final interpreter path. - Setting exec_is_script = true on the first confirmed shebang (rc > 0 from elf_read_shebang), so deeply nested chains also trip the flag. - After elf_load, applying S_ISUID/S_ISGID only when the directly-executed file was a regular non-script ELF. - Honouring the kernel's S_ISGID-requires-S_IXGRP rule to distinguish mandatory-locking GIDs from setgid execution. Fix sysprog21#138
There was a problem hiding this comment.
4 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/syscall/exec.c">
<violation number="1" location="src/syscall/exec.c:431">
P0: Replacing a set-id target between this `stat()` and the later pathname opens can run attacker-selected ELF bytes with the replaced file's effective IDs. Bind the executable to an fd, derive metadata with `fstat`, and load/map that same fd.</violation>
<violation number="2" location="src/syscall/exec.c:577">
P1: A readable but non-executable setuid ELF is accepted by the loader and receives the file owner's effective UID. Require Linux execute permission for the target and interpreter before applying set-id credentials or loading the image.</violation>
<violation number="3" location="src/syscall/exec.c:585">
P1: A normal exec after dropping set-id privilege retains the previous saved ID, letting the replacement program regain it with `setuid()`. Refresh saved UID/GID from the final effective IDs on every successful exec, not only when `cred_changed` is true.</violation>
<violation number="4" location="src/syscall/exec.c:586">
P0: A rejected set-id exec can return to the old program with elevated credentials, e.g. when its PT_INTERP is invalid or Rosetta is disabled. Commit credentials only after all recoverable validation succeeds, immediately before the point of no return.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| */ | ||
| bool exec_is_script = false; | ||
| struct stat exec_st; | ||
| bool have_exec_st = (stat(path_host, &exec_st) == 0); |
There was a problem hiding this comment.
P0: Replacing a set-id target between this stat() and the later pathname opens can run attacker-selected ELF bytes with the replaced file's effective IDs. Bind the executable to an fd, derive metadata with fstat, and load/map that same fd.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/exec.c, line 431:
<comment>Replacing a set-id target between this `stat()` and the later pathname opens can run attacker-selected ELF bytes with the replaced file's effective IDs. Bind the executable to an fd, derive metadata with `fstat`, and load/map that same fd.</comment>
<file context>
@@ -421,6 +422,14 @@ int64_t sys_execve(hv_vcpu_t vcpu,
+ */
+ bool exec_is_script = false;
+ struct stat exec_st;
+ bool have_exec_st = (stat(path_host, &exec_st) == 0);
+
while (true) {
</file context>
| cred_changed = true; | ||
| } | ||
| if (cred_changed) { | ||
| proc_set_ids(proc_get_uid(), new_euid, new_euid, proc_get_gid(), |
There was a problem hiding this comment.
P0: A rejected set-id exec can return to the old program with elevated credentials, e.g. when its PT_INTERP is invalid or Rosetta is disabled. Commit credentials only after all recoverable validation succeeds, immediately before the point of no return.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/exec.c, line 586:
<comment>A rejected set-id exec can return to the old program with elevated credentials, e.g. when its PT_INTERP is invalid or Rosetta is disabled. Commit credentials only after all recoverable validation succeeds, immediately before the point of no return.</comment>
<file context>
@@ -551,6 +563,31 @@ int64_t sys_execve(hv_vcpu_t vcpu,
+ cred_changed = true;
+ }
+ if (cred_changed) {
+ proc_set_ids(proc_get_uid(), new_euid, new_euid, proc_get_gid(),
+ new_egid, new_egid);
+ }
</file context>
| new_egid = (uint32_t) exec_st.st_gid; | ||
| cred_changed = true; | ||
| } | ||
| if (cred_changed) { |
There was a problem hiding this comment.
P1: A normal exec after dropping set-id privilege retains the previous saved ID, letting the replacement program regain it with setuid(). Refresh saved UID/GID from the final effective IDs on every successful exec, not only when cred_changed is true.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/exec.c, line 585:
<comment>A normal exec after dropping set-id privilege retains the previous saved ID, letting the replacement program regain it with `setuid()`. Refresh saved UID/GID from the final effective IDs on every successful exec, not only when `cred_changed` is true.</comment>
<file context>
@@ -551,6 +563,31 @@ int64_t sys_execve(hv_vcpu_t vcpu,
+ new_egid = (uint32_t) exec_st.st_gid;
+ cred_changed = true;
+ }
+ if (cred_changed) {
+ proc_set_ids(proc_get_uid(), new_euid, new_euid, proc_get_gid(),
+ new_egid, new_egid);
</file context>
| uint32_t new_euid = proc_get_euid(); | ||
| uint32_t new_egid = proc_get_egid(); | ||
| bool cred_changed = false; | ||
| if (exec_st.st_mode & S_ISUID) { |
There was a problem hiding this comment.
P1: A readable but non-executable setuid ELF is accepted by the loader and receives the file owner's effective UID. Require Linux execute permission for the target and interpreter before applying set-id credentials or loading the image.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/syscall/exec.c, line 577:
<comment>A readable but non-executable setuid ELF is accepted by the loader and receives the file owner's effective UID. Require Linux execute permission for the target and interpreter before applying set-id credentials or loading the image.</comment>
<file context>
@@ -551,6 +563,31 @@ int64_t sys_execve(hv_vcpu_t vcpu,
+ uint32_t new_euid = proc_get_euid();
+ uint32_t new_egid = proc_get_egid();
+ bool cred_changed = false;
+ if (exec_st.st_mode & S_ISUID) {
+ new_euid = (uint32_t) exec_st.st_uid;
+ cred_changed = true;
</file context>
The Linux kernel (fs/exec.c bprm_fill_uid) deliberately ignores setuid/setgid bits on interpreted scripts to prevent privilege escalation via interpreter manipulation. elfuse had no such guard: exec_is_script was never tracked, so the setuid/setgid check was effectively always true for all executables.
Fix by:
Fix #138
Summary by cubic
Suppress setuid/setgid on shebang scripts to match Linux behavior and prevent privilege escalation. Apply suid/sgid only when the originally executed file is a regular non-script ELF, and require group-exec for sgid.
exec_is_scripton the first confirmed shebang, including nested chains.Written for commit d7a47ff. Summary will update on new commits.