From 3737056377c595663c5eee47c6609afb211bc732 Mon Sep 17 00:00:00 2001 From: Petr Lautrbach Date: Tue, 3 Feb 2026 13:55:30 +0100 Subject: [PATCH] policycoreutils-3.6-5 - sandbox/seunshare: Replace system() with execv() to prevent shell injection - seunshare: fix the frail tmpdir cleanup Resolves: RHEL-144055 --- ...lways-use-translations-when-printing.patch | 79 ++++++ ...unshare-fix-the-frail-tmpdir-cleanup.patch | 133 +++++++++ ...e-Replace-system-with-execv-to-preve.patch | 257 ++++++++++++++++++ policycoreutils.spec | 9 +- 4 files changed, 477 insertions(+), 1 deletion(-) create mode 100644 0023-seunshare-always-use-translations-when-printing.patch create mode 100644 0024-seunshare-fix-the-frail-tmpdir-cleanup.patch create mode 100644 0025-sandbox-seunshare-Replace-system-with-execv-to-preve.patch diff --git a/0023-seunshare-always-use-translations-when-printing.patch b/0023-seunshare-always-use-translations-when-printing.patch new file mode 100644 index 0000000..42cf8dc --- /dev/null +++ b/0023-seunshare-always-use-translations-when-printing.patch @@ -0,0 +1,79 @@ +From fe5706639f4a4c8e331274ad47cf0c5a36371f76 Mon Sep 17 00:00:00 2001 +From: Rahul Sandhu +Date: Thu, 31 Jul 2025 18:17:06 +0100 +Subject: [PATCH] seunshare: always use translations when printing +Content-type: text/plain + +Some errors previously were not using gettext for translations, hence +wrap them with the _ macro. + +Signed-off-by: Rahul Sandhu +Acked-by: Stephen Smalley +--- + sandbox/seunshare.c | 14 +++++++------- + 1 file changed, 7 insertions(+), 7 deletions(-) + +diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c +index 1d38ea92b9ae..9bce167aeac7 100644 +--- a/sandbox/seunshare.c ++++ b/sandbox/seunshare.c +@@ -102,7 +102,7 @@ static int set_signal_handles(void) + + /* Empty the signal mask in case someone is blocking a signal */ + if (sigemptyset(&empty)) { +- fprintf(stderr, "Unable to obtain empty signal set\n"); ++ fprintf(stderr, _("Unable to obtain empty signal set\n")); + return -1; + } + +@@ -110,12 +110,12 @@ static int set_signal_handles(void) + + /* Terminate on SIGHUP */ + if (signal(SIGHUP, SIG_DFL) == SIG_ERR) { +- perror("Unable to set SIGHUP handler"); ++ perror(_("Unable to set SIGHUP handler")); + return -1; + } + + if (signal(SIGINT, handler) == SIG_ERR) { +- perror("Unable to set SIGINT handler"); ++ perror(_("Unable to set SIGINT handler")); + return -1; + } + +@@ -323,7 +323,7 @@ static int rsynccmd(const char * src, const char *dst, char **cmdbuf) + + /* match glob for all files in src dir */ + if (asprintf(&buf, "%s/*", src) == -1) { +- fprintf(stderr, "Out of memory\n"); ++ fprintf(stderr, _("Out of memory\n")); + return -1; + } + +@@ -341,12 +341,12 @@ static int rsynccmd(const char * src, const char *dst, char **cmdbuf) + + if (!buf) { + if (asprintf(&newbuf, "\'%s\'", path) == -1) { +- fprintf(stderr, "Out of memory\n"); ++ fprintf(stderr, _("Out of memory\n")); + goto err; + } + } else { + if (asprintf(&newbuf, "%s \'%s\'", buf, path) == -1) { +- fprintf(stderr, "Out of memory\n"); ++ fprintf(stderr, _("Out of memory\n")); + goto err; + } + } +@@ -357,7 +357,7 @@ static int rsynccmd(const char * src, const char *dst, char **cmdbuf) + + if (buf) { + if (asprintf(&newbuf, "/usr/bin/rsync -trlHDq %s '%s'", buf, dst) == -1) { +- fprintf(stderr, "Out of memory\n"); ++ fprintf(stderr, _("Out of memory\n")); + goto err; + } + *cmdbuf=newbuf; +-- +2.52.0 + diff --git a/0024-seunshare-fix-the-frail-tmpdir-cleanup.patch b/0024-seunshare-fix-the-frail-tmpdir-cleanup.patch new file mode 100644 index 0000000..7a717da --- /dev/null +++ b/0024-seunshare-fix-the-frail-tmpdir-cleanup.patch @@ -0,0 +1,133 @@ +From ea48acce4ff2bb6267b038c0127d9cc8b2c5b021 Mon Sep 17 00:00:00 2001 +From: Rahul Sandhu +Date: Tue, 7 Oct 2025 19:09:06 +0100 +Subject: [PATCH] seunshare: fix the frail tmpdir cleanup +Content-type: text/plain + +For some reason, rm is invoked via system (3) to cleanup the runtime +temp directory. This really isn't all that robust, *especially* given +that seunshare is supposed to be a security boundary. Instead do this +using libc, the API designed to be used within C programs. + +Also make sure that we don't follow symbolic links; the input being +deleted is untrusted, and hence a malicious symbolic link may be placed +outside of the sandbox. + +Signed-off-by: Rahul Sandhu +Acked-by: Stephen Smalley +--- + sandbox/seunshare.c | 78 ++++++++++++++++++++++++++++++++++++++------- + 1 file changed, 66 insertions(+), 12 deletions(-) + +diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c +index 9bce167aeac7..f0f22f797f84 100644 +--- a/sandbox/seunshare.c ++++ b/sandbox/seunshare.c +@@ -4,6 +4,7 @@ + */ + + #define _GNU_SOURCE ++#include + #include + #include + #include +@@ -373,6 +374,66 @@ err: + return rc; + } + ++/* ++ * Recursively delete a directory. ++ * SAFETY: This function will NOT follow symbolic links (AT_SYMLINK_NOFOLLOW). ++ * As a result, this function can be run safely on a directory owned by ++ * a non-root user: symbolic links to root paths (such as /root) will ++ * not be followed. ++ */ ++static bool rm_rf(int targetfd, const char *path) { ++ struct stat statbuf; ++ ++ if (fstatat(targetfd, path, &statbuf, AT_SYMLINK_NOFOLLOW) < 0) { ++ if (errno == ENOENT) { ++ return true; ++ } ++ perror("fstatat"); ++ return false; ++ } ++ ++ if (S_ISDIR(statbuf.st_mode)) { ++ const int newfd = openat(targetfd, path, O_RDONLY | O_DIRECTORY | O_CLOEXEC); ++ if (newfd < 0) { ++ perror("openat"); ++ return false; ++ } ++ ++ DIR *dir = fdopendir(newfd); ++ if (!dir) { ++ perror("fdopendir"); ++ close(newfd); ++ return false; ++ } ++ ++ struct dirent *entry; ++ int rc = true; ++ while ((entry = readdir(dir)) != NULL) { ++ if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) { ++ continue; ++ } ++ ++ if (!rm_rf(dirfd(dir), entry->d_name)) { ++ rc = false; ++ } ++ } ++ ++ closedir(dir); ++ ++ if (unlinkat(targetfd, path, AT_REMOVEDIR) < 0) { ++ perror("unlinkat"); ++ rc = false; ++ } ++ ++ return rc; ++ } ++ if (unlinkat(targetfd, path, 0) < 0) { ++ perror("unlinkat"); ++ return false; ++ } ++ return true; ++} ++ + /** + * Clean up runtime temporary directory. Returns 0 if no problem was detected, + * >0 if some error was detected, but errors here are treated as non-fatal and +@@ -398,24 +459,17 @@ static int cleanup_tmpdir(const char *tmpdir, const char *src, + free(cmdbuf); cmdbuf = NULL; + } + +- /* remove files from the runtime temporary directory */ +- if (asprintf(&cmdbuf, "/bin/rm -r '%s/' 2>/dev/null", tmpdir) == -1) { +- fprintf(stderr, _("Out of memory\n")); +- cmdbuf = NULL; ++ if ((uid_t)setfsuid(0) != 0) { ++ /* setfsuid does not return error, but this check makes code checkers happy */ + rc++; + } +- /* this may fail if there's root-owned file left in the runtime tmpdir */ +- if (cmdbuf && spawn_command(cmdbuf, pwd->pw_uid) != 0) rc++; +- free(cmdbuf); cmdbuf = NULL; + +- /* remove runtime temporary directory */ +- if ((uid_t)setfsuid(0) != 0) { +- /* setfsuid does not return error, but this check makes code checkers happy */ ++ /* Recursively remove the runtime temp directory. */ ++ if (!rm_rf(AT_FDCWD, tmpdir)) { ++ fprintf(stderr, _("Failed to recursively remove directory %s\n"), tmpdir); + rc++; + } + +- if (pwd->pw_uid != 0 && rmdir(tmpdir) == -1) +- fprintf(stderr, _("Failed to remove directory %s: %s\n"), tmpdir, strerror(errno)); + if ((uid_t)setfsuid(pwd->pw_uid) != 0) { + fprintf(stderr, _("unable to switch back to user after clearing tmp dir\n")); + rc++; +-- +2.52.0 + diff --git a/0025-sandbox-seunshare-Replace-system-with-execv-to-preve.patch b/0025-sandbox-seunshare-Replace-system-with-execv-to-preve.patch new file mode 100644 index 0000000..cd9b704 --- /dev/null +++ b/0025-sandbox-seunshare-Replace-system-with-execv-to-preve.patch @@ -0,0 +1,257 @@ +From 9ad5e7f2b8d219c1ae187f8b3c664458ce7de32f Mon Sep 17 00:00:00 2001 +From: Petr Lautrbach +Date: Wed, 7 Jan 2026 17:58:34 +0100 +Subject: [PATCH] sandbox/seunshare: Replace system() with execv() to prevent + shell injection +Content-type: text/plain + +Refactor spawn_command() to use execv() instead of system() to eliminate +shell injection vulnerabilities. + +Reported-By: Rahul Sandhu +Signed-off-by: Petr Lautrbach +Acked-by: James Carter +Signed-off-by: Jason Zaman +--- + sandbox/seunshare.c | 132 ++++++++++++++++++++++++++------------------ + 1 file changed, 79 insertions(+), 53 deletions(-) + +diff --git a/sandbox/seunshare.c b/sandbox/seunshare.c +index f0f22f797f84..b49fc2cd8050 100644 +--- a/sandbox/seunshare.c ++++ b/sandbox/seunshare.c +@@ -55,6 +55,12 @@ + #define DEFAULT_PATH "/usr/bin:/bin" + #define USAGE_STRING _("USAGE: seunshare [ -v ] [ -C ] [ -k ] [ -t tmpdir ] [ -h homedir ] [ -r runuserdir ] [ -Z CONTEXT ] -- executable [args] ") + ++#define strdup_or_err(args, index, src) do { \ ++ args[index] = strdup(src); \ ++ if (! args[index]) \ ++ goto err; \ ++ } while(0) ++ + static int verbose = 0; + static int child = 0; + +@@ -135,15 +141,14 @@ static int set_signal_handles(void) + } while(0) + + /** +- * Spawn external command using system() with dropped privileges. +- * TODO: avoid system() and use exec*() instead ++ * Spawn external command with dropped privileges. + */ +-static int spawn_command(const char *cmd, uid_t uid){ ++static int spawn_command(char **cmd, uid_t uid){ + int childpid; + int status = -1; + + if (verbose > 1) +- printf("spawn_command: %s\n", cmd); ++ printf("spawn_command: %s\n", cmd[0]); + + childpid = fork(); + if (childpid == -1) { +@@ -154,8 +159,7 @@ static int spawn_command(const char *cmd, uid_t uid){ + if (childpid == 0) { + if (drop_privs(uid) != 0) exit(-1); + +- status = system(cmd); +- status_to_retval(status, status); ++ status = execv(cmd[0], cmd); + exit(status); + } + +@@ -312,15 +316,24 @@ static int bad_path(const char *path) { + return 0; + } + +-static int rsynccmd(const char * src, const char *dst, char **cmdbuf) +-{ ++static void free_args(char **args) { ++ char **args_p = args; ++ if (! args) ++ return; ++ while (*args_p != NULL) { ++ free(*args_p); ++ args_p++; ++ } ++ free(args); ++} ++ ++static int rsynccmd(const char * src, const char *dst, char ***cmd) { ++ char **args; + char *buf = NULL; +- char *newbuf = NULL; + glob_t fglob; + fglob.gl_offs = 0; + int flags = GLOB_PERIOD; +- unsigned int i = 0; +- int rc = -1; ++ unsigned int i = 0, index; + + /* match glob for all files in src dir */ + if (asprintf(&buf, "%s/*", src) == -1) { +@@ -335,43 +348,35 @@ static int rsynccmd(const char * src, const char *dst, char **cmdbuf) + + free(buf); buf = NULL; + +- for ( i=0; i < fglob.gl_pathc; i++) { +- const char *path = fglob.gl_pathv[i]; +- +- if (bad_path(path)) continue; +- +- if (!buf) { +- if (asprintf(&newbuf, "\'%s\'", path) == -1) { +- fprintf(stderr, _("Out of memory\n")); +- goto err; +- } +- } else { +- if (asprintf(&newbuf, "%s \'%s\'", buf, path) == -1) { +- fprintf(stderr, _("Out of memory\n")); +- goto err; +- } +- } +- +- free(buf); buf = newbuf; +- newbuf = NULL; ++ /* rsync -trlHDq + + dst + NULL */ ++ *cmd = calloc(2 + fglob.gl_pathc + 2, sizeof(char *)); ++ if (! *cmd) { ++ fprintf(stderr, _("Out of memory\n")); ++ return -1; + } + +- if (buf) { +- if (asprintf(&newbuf, "/usr/bin/rsync -trlHDq %s '%s'", buf, dst) == -1) { +- fprintf(stderr, _("Out of memory\n")); +- goto err; +- } +- *cmdbuf=newbuf; +- } +- else { +- *cmdbuf=NULL; +- } +- rc = 0; ++ args = *cmd; ++ strdup_or_err(args, 0, "/usr/bin/rsync"); ++ strdup_or_err(args, 1, "-trlHDq"); + ++ for ( i=0, index = 2; i < fglob.gl_pathc; i++) { ++ const char *path = fglob.gl_pathv[i]; ++ if (bad_path(path)) continue; ++ strdup_or_err(args, index, path); ++ index++; ++ } ++ strdup_or_err(args, index, dst); ++ index++; ++ args[index] = NULL; ++ globfree(&fglob); ++ return 0; + err: +- free(buf); buf = NULL; + globfree(&fglob); +- return rc; ++ if (args) { ++ free_args(args); ++ *cmd = NULL; ++ } ++ return -1; + } + + /* +@@ -442,21 +447,38 @@ static bool rm_rf(int targetfd, const char *path) { + static int cleanup_tmpdir(const char *tmpdir, const char *src, + struct passwd *pwd, int copy_content) + { +- char *cmdbuf = NULL; ++ char **args; + int rc = 0; + + /* rsync files back */ + if (copy_content) { +- if (asprintf(&cmdbuf, "/usr/bin/rsync --exclude=.X11-unix -utrlHDq --delete '%s/' '%s/'", tmpdir, src) == -1) { ++ args = calloc(7, sizeof(char *)); ++ if (! args) { + fprintf(stderr, _("Out of memory\n")); +- cmdbuf = NULL; +- rc++; ++ return 1; ++ } ++ ++ strdup_or_err(args, 0, "/usr/bin/rsync"); ++ strdup_or_err(args, 1, "--exclude=.X11-unix"); ++ strdup_or_err(args, 2, "-utrlHDq"); ++ strdup_or_err(args, 3, "--delete"); ++ if (asprintf(&args[4], "%s/", tmpdir) == -1) { ++ fprintf(stderr, _("Out of memory\n")); ++ free_args(args); ++ return 1; + } +- if (cmdbuf && spawn_command(cmdbuf, pwd->pw_uid) != 0) { ++ if (asprintf(&args[5], "%s/", src) == -1) { ++ fprintf(stderr, _("Out of memory\n")); ++ free_args(args); ++ return 1; ++ } ++ args[6] = NULL; ++ ++ if (spawn_command(args, pwd->pw_uid) != 0) { + fprintf(stderr, _("Failed to copy files from the runtime temporary directory\n")); + rc++; + } +- free(cmdbuf); cmdbuf = NULL; ++ free_args(args); + } + + if ((uid_t)setfsuid(0) != 0) { +@@ -476,6 +498,10 @@ static int cleanup_tmpdir(const char *tmpdir, const char *src, + } + + return rc; ++err: ++ if (args) ++ free_args(args); ++ return 1; + } + + /** +@@ -488,7 +514,7 @@ static char *create_tmpdir(const char *src, struct stat *src_st, + struct stat *out_st, struct passwd *pwd, const char *execcon) + { + char *tmpdir = NULL; +- char *cmdbuf = NULL; ++ char **cmd = NULL; + int fd_t = -1, fd_s = -1; + struct stat tmp_st; + char *con = NULL; +@@ -575,7 +601,7 @@ static char *create_tmpdir(const char *src, struct stat *src_st, + if ((uid_t)setfsuid(pwd->pw_uid) != 0) + goto err; + +- if (rsynccmd(src, tmpdir, &cmdbuf) < 0) { ++ if (rsynccmd(src, tmpdir, &cmd) < 0) { + goto err; + } + +@@ -583,7 +609,7 @@ static char *create_tmpdir(const char *src, struct stat *src_st, + if ((uid_t)setfsuid(0) != pwd->pw_uid) + goto err; + +- if (cmdbuf && spawn_command(cmdbuf, pwd->pw_uid) != 0) { ++ if (spawn_command(cmd, pwd->pw_uid) != 0) { + fprintf(stderr, _("Failed to populate runtime temporary directory\n")); + cleanup_tmpdir(tmpdir, src, pwd, 0); + goto err; +@@ -593,7 +619,7 @@ static char *create_tmpdir(const char *src, struct stat *src_st, + err: + free(tmpdir); tmpdir = NULL; + good: +- free(cmdbuf); cmdbuf = NULL; ++ free_args(cmd); + freecon(con); con = NULL; + if (fd_t >= 0) close(fd_t); + if (fd_s >= 0) close(fd_s); +-- +2.52.0 + diff --git a/policycoreutils.spec b/policycoreutils.spec index 518d4af..e6706b9 100644 --- a/policycoreutils.spec +++ b/policycoreutils.spec @@ -11,7 +11,7 @@ Summary: SELinux policy core utilities Name: policycoreutils Version: 3.6 -Release: 4%{?dist} +Release: 5%{?dist} License: GPL-2.0-or-later # https://github.com/SELinuxProject/selinux/wiki/Releases Source0: https://github.com/SELinuxProject/selinux/releases/download/3.6/selinux-3.6.tar.gz @@ -58,6 +58,9 @@ Patch0019: 0019-python-semanage-Do-not-sort-local-fcontext-definitio.patch Patch0020: 0020-fixfiles-drop-unnecessary-line-endings.patch Patch0021: 0021-restorecond-always-add-0-to-ut_user.patch Patch0022: 0022-semanage-Reset-active-value-when-deleting-boolean-cu.patch +Patch0023: 0023-seunshare-always-use-translations-when-printing.patch +Patch0024: 0024-seunshare-fix-the-frail-tmpdir-cleanup.patch +Patch0025: 0025-sandbox-seunshare-Replace-system-with-execv-to-preve.patch # Patch list end Obsoletes: policycoreutils < 2.0.61-2 Conflicts: filesystem < 3, selinux-policy-base < 3.13.1-138 @@ -467,6 +470,10 @@ The policycoreutils-restorecond package contains the restorecond service. %systemd_postun_with_restart restorecond.service %changelog +* Tue Feb 03 2026 Petr Lautrbach - 3.6-5 +- sandbox/seunshare: Replace system() with execv() to prevent shell injection +- seunshare: fix the frail tmpdir cleanup + * Thu Jan 15 2026 Veronika Syncakova - 3.6-4 - semanage: Reset active value when deleting boolean customizations