diff --git a/gprofiler/metadata/versions.py b/gprofiler/metadata/versions.py index 5a3cd767c..b3fb22503 100644 --- a/gprofiler/metadata/versions.py +++ b/gprofiler/metadata/versions.py @@ -19,7 +19,7 @@ from granulate_utils.linux.ns import get_process_nspid, run_in_ns_wrapper from psutil import NoSuchProcess, Process -from gprofiler.utils import run_process +from gprofiler.utils import run_process_as_target def get_exe_version( @@ -30,13 +30,25 @@ def get_exe_version( try_stderr: bool = False, ) -> str: """ - Runs {process.exe()} --version in the appropriate namespace + Runs {process.exe()} --version in the appropriate namespace. + + Security: Executes with the target process's UID/GID to prevent + privilege escalation if the binary is attacker-controlled. """ exe_path = f"/proc/{get_process_nspid(process.pid)}/exe" + # Get credentials BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace) + target_uid = process.uids().real + target_gid = process.gids().real + def _run_get_version() -> "CompletedProcess[bytes]": - return run_process( - [exe_path, version_arg], stop_event=stop_event, timeout=get_version_timeout, pdeathsigger=False + return run_process_as_target( + [exe_path, version_arg], + target_uid=target_uid, + target_gid=target_gid, + stop_event=stop_event, + timeout=get_version_timeout, ) try: diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index 2e85486a1..8f79b3292 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -95,6 +95,7 @@ remove_prefix, resource_path, run_process, + run_process_as_target, touch_path, wait_event, ) @@ -353,20 +354,32 @@ def _get_process_ns_java_path(process: Process) -> Optional[str]: # process is hashable and the same process instance compares equal @functools.lru_cache(maxsize=_JAVA_VERSION_CACHE_MAX) def get_java_version(process: Process, stop_event: Event) -> Optional[str]: + """ + Get Java version from the process's java binary. + + Security: Executes with the target process's UID/GID to prevent + privilege escalation if the binary is attacker-controlled. + """ # make sure we're able to find "java" binary bundled with process libjvm process_java_path = _get_process_ns_java_path(process) if process_java_path is None: return None + # Get credentials BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace) + target_uid = process.uids().real + target_gid = process.gids().real + def _run_java_version() -> "CompletedProcess[bytes]": - return run_process( + return run_process_as_target( [ process_java_path, "-version", ], + target_uid=target_uid, + target_gid=target_gid, stop_event=stop_event, timeout=_JAVA_VERSION_TIMEOUT, - pdeathsigger=False, ) # doesn't work without changing PID NS as well (I'm getting ENOENT for libjli.so) diff --git a/gprofiler/profilers/python.py b/gprofiler/profilers/python.py index 781fb1b40..fc074210c 100644 --- a/gprofiler/profilers/python.py +++ b/gprofiler/profilers/python.py @@ -61,7 +61,15 @@ if is_linux(): from gprofiler.profilers.python_ebpf import PythonEbpfProfiler, PythonEbpfError -from gprofiler.utils import pgrep_exe, pgrep_maps, random_prefix, removed_path, resource_path, run_process +from gprofiler.utils import ( + pgrep_exe, + pgrep_maps, + random_prefix, + removed_path, + resource_path, + run_process, + run_process_as_target, +) from gprofiler.utils.process import process_comm, search_proc_maps logger = get_logger_adapter(__name__) @@ -131,6 +139,12 @@ def _get_python_version(self, process: Process) -> Optional[str]: return None def _get_sys_maxunicode(self, process: Process) -> Optional[str]: + """ + Get sys.maxunicode from a Python 2 process. + + Security: Executes with the target process's UID/GID to prevent + privilege escalation if the binary is attacker-controlled. + """ try: if not is_process_basename_matching(process, application_identifiers._PYTHON_BIN_RE): # see same raise above @@ -138,12 +152,18 @@ def _get_sys_maxunicode(self, process: Process) -> Optional[str]: python_path = f"/proc/{get_process_nspid(process.pid)}/exe" + # Get credentials BEFORE entering namespace + # (psutil can't resolve host PIDs inside namespace) + target_uid = process.uids().real + target_gid = process.gids().real + def _run_python_process_in_ns() -> "CompletedProcess[bytes]": - return run_process( + return run_process_as_target( [python_path, "-S", "-c", "import sys; print(sys.maxunicode)"], + target_uid=target_uid, + target_gid=target_gid, stop_event=self._stop_event, timeout=self._PYTHON_TIMEOUT, - pdeathsigger=False, ) result = cast(CompletedProcess, run_in_ns_wrapper(["pid", "mnt"], _run_python_process_in_ns, process.pid)) diff --git a/gprofiler/utils/__init__.py b/gprofiler/utils/__init__.py index 91aaf4eb8..eafad10dd 100644 --- a/gprofiler/utils/__init__.py +++ b/gprofiler/utils/__init__.py @@ -59,6 +59,7 @@ StopEventSetException, ) from gprofiler.log import get_logger_adapter +from gprofiler.platform import is_linux as _is_linux_for_priv logger = get_logger_adapter(__name__) @@ -525,6 +526,81 @@ def cleanup_process_reference(process: Popen) -> None: pass # Already removed +def run_process_as_target( + cmd: List[str], + target_uid: int, + target_gid: int, + stop_event: Optional[Event] = None, + timeout: int = 5, + **kwargs: Any, +) -> "CompletedProcess[bytes]": + """ + Execute a command with the specified UID/GID (dropping privileges if root). + + Security: This function drops privileges before executing the command, + preventing privilege escalation if the binary is attacker-controlled. + + Note: Callers must resolve target_uid/target_gid BEFORE entering any + PID/mount namespaces, since these lookups may fail inside a different namespace. + + Uses subprocess user/group parameters which handle privilege dropping in + the child process after fork (implemented in C, no preexec_fn deadlock risk). + + Args: + cmd: Command and arguments to execute + target_uid: The UID to run the command as + target_gid: The GID to run the command as + stop_event: Optional event to signal stop (created internally if None) + timeout: Command timeout in seconds (default: 5) + **kwargs: Additional arguments passed to run_process() + + Returns: + CompletedProcess with stdout/stderr + """ + # Create a dummy Event if none provided, so timeouts work + # (run_process asserts timeout must be None when stop_event is None) + if stop_event is None: + stop_event = Event() + + # Always disable pdeathsigger wrapper when running as target + # This function is called inside target namespaces where pdeathsigger may not exist + kwargs["pdeathsigger"] = False + + # Only drop privileges on Linux when running as root and target is non-root + # Use is_root() which handles user-namespace/container scenarios properly + should_drop = _is_linux_for_priv() and is_root() and target_uid != 0 + logger.debug( + "run_process_as_target called", + cmd=cmd, + target_uid=target_uid, + target_gid=target_gid, + is_linux=_is_linux_for_priv(), + is_root=is_root(), + should_drop_privileges=should_drop, + ) + + if should_drop: + # Use subprocess user/group params for privilege dropping + # This is handled in C code after fork(), before exec() - no deadlock risk + # and works inside any namespace (no external binary needed) + kwargs["user"] = target_uid + kwargs["group"] = target_gid + kwargs["extra_groups"] = [] + logger.debug( + "Privilege dropping enabled", + user=kwargs["user"], + group=kwargs["group"], + ) + + logger.debug("Calling run_process", cmd=cmd, timeout=timeout) + return run_process( + cmd, + stop_event=stop_event, + timeout=timeout, + **kwargs, + ) + + def _exit_handler() -> None: for process in _processes: process.kill() diff --git a/scripts/pdeathsigger.c b/scripts/pdeathsigger.c index fe8f5b98e..17b1c3363 100644 --- a/scripts/pdeathsigger.c +++ b/scripts/pdeathsigger.c @@ -1,27 +1,87 @@ #include #include +#include #include #include #include +#include +#include /* preexec_fn is not safe to use in the presence of threads, child process could deadlock before exec is called. - this little shim is a workaround to avoid using preexe_fn and - still get the desired behavior (PR_SET_PDEATHSIG). + this little shim is a workaround to avoid using preexec_fn and + still get the desired behavior (PR_SET_PDEATHSIG and privilege dropping). + + Usage: + pdeathsigger /path/to/binary [args...] + pdeathsigger -u -g /path/to/binary [args...] + + When -u and -g are provided, drops privileges to the specified UID/GID + before executing the command. This prevents privilege escalation if the + target binary is attacker-controlled. */ int main(int argc, char *argv[]) { - if (argc < 2) { - fprintf(stderr, "Usage: %s /path/to/binary [args...]\n", argv[0]); + int arg_offset = 1; + long uid = -1; + long gid = -1; + char *endptr; + + /* Parse optional -u and -g flags */ + while (arg_offset < argc) { + if (strcmp(argv[arg_offset], "-u") == 0 && arg_offset + 1 < argc) { + errno = 0; + uid = strtol(argv[arg_offset + 1], &endptr, 10); + if (errno != 0 || *endptr != '\0' || uid < 0) { + fprintf(stderr, "Invalid UID: %s\n", argv[arg_offset + 1]); + return 1; + } + arg_offset += 2; + } else if (strcmp(argv[arg_offset], "-g") == 0 && arg_offset + 1 < argc) { + errno = 0; + gid = strtol(argv[arg_offset + 1], &endptr, 10); + if (errno != 0 || *endptr != '\0' || gid < 0) { + fprintf(stderr, "Invalid GID: %s\n", argv[arg_offset + 1]); + return 1; + } + arg_offset += 2; + } else { + break; + } + } + + if (arg_offset >= argc) { + fprintf(stderr, "Usage: %s [-u -g ] /path/to/binary [args...]\n", argv[0]); return 1; } + /* Set PR_SET_PDEATHSIG */ if (prctl(PR_SET_PDEATHSIG, SIGKILL) == -1) { perror("prctl"); return 1; } - execvp(argv[1], &argv[1]); + /* Drop privileges if uid/gid were specified and we're root */ + if (uid >= 0 && gid >= 0 && geteuid() == 0 && uid != 0) { + /* Drop supplementary groups */ + if (setgroups(0, NULL) == -1) { + perror("setgroups"); + return 1; + } + + /* Set GID before UID (can't change GID after dropping root) */ + if (setgid((gid_t)gid) == -1) { + perror("setgid"); + return 1; + } + + if (setuid((uid_t)uid) == -1) { + perror("setuid"); + return 1; + } + } + + execvp(argv[arg_offset], &argv[arg_offset]); perror("execvp"); return 1;