Skip to content

Suppress setuid and setgid on shebang scripts #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
doanbaotrung wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Suppress setuid and setgid on shebang scripts #199

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/syscall/exec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <sys/stat.h>
#include <libkern/OSCacheControl.h>

#include "debug/log.h"
Expand Down Expand Up @@ -421,6 +422,14 @@ int64_t sys_execve(hv_vcpu_t vcpu,
elf_info_t elf_info;
int shebang_depth = 0;

/* Snapshot the directly-executed file's metadata before shebang
* resolution overwrites path_host with the interpreter path. The
* setuid/setgid check below needs the *original* file's mode bits.
*/
bool exec_is_script = false;
struct stat exec_st;
bool have_exec_st = (stat(path_host, &exec_st) == 0);

@cubic-dev-ai cubic-dev-ai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>


while (true) {
char interp_start[256];
char interp_arg[256];
Expand All @@ -434,6 +443,9 @@ int64_t sys_execve(hv_vcpu_t vcpu,
if (rc == 0)
break;

/* The file at the current path is a shebang script. */
exec_is_script = true;

/* The current path is a script. Bound the resolution chain only once a
* further shebang is confirmed, so a max-depth chain ending in a real
* ELF still loads (matches the prior elf_load-first loop).
Expand Down Expand Up @@ -551,6 +563,31 @@ int64_t sys_execve(hv_vcpu_t vcpu,
goto fail;
}

/* Apply 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.
*/
if (have_exec_st && !exec_is_script && S_ISREG(exec_st.st_mode)) {
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) {

@cubic-dev-ai cubic-dev-ai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

new_euid = (uint32_t) exec_st.st_uid;
cred_changed = true;
}
if ((exec_st.st_mode & S_ISGID) && (exec_st.st_mode & S_IXGRP)) {
new_egid = (uint32_t) exec_st.st_gid;
cred_changed = true;
}
if (cred_changed) {

@cubic-dev-ai cubic-dev-ai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

proc_set_ids(proc_get_uid(), new_euid, new_euid, proc_get_gid(),

@cubic-dev-ai cubic-dev-ai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, new_egid);
}
}

/* Pre-PNR validation. All checks that can fail gracefully MUST happen
* before guest_reset(). After guest_reset(), the old process image is gone.
* Failures are unrecoverable, matching the Linux kernel's behavior
Expand Down
Loading