From 86328e78ea03f0c96d571b48fa79df014dfb40e9 Mon Sep 17 00:00:00 2001 From: Karel Zak Date: Mon, 21 Jun 2021 12:25:31 +0200 Subject: [PATCH] include/c: add drop_permissions(), consolidate UID/GID reset Fixes: https://github.com/karelzak/util-linux/issues/1354 Signed-off-by: Karel Zak --- include/c.h | 24 ++++++++++++++++++++++++ lib/canonicalize.c | 3 +-- libblkid/src/topology/dm.c | 5 +---- libblkid/src/topology/lvm.c | 5 +---- libmount/src/context_mount.c | 5 +---- libmount/src/context_umount.c | 5 +---- sys-utils/eject.c | 8 ++------ sys-utils/mount.c | 9 ++------- sys-utils/swapon.c | 9 ++------- sys-utils/umount.c | 9 ++------- text-utils/more.c | 9 +++------ 11 files changed, 40 insertions(+), 51 deletions(-) diff --git a/include/c.h b/include/c.h index e7842c106..c1e4c5ffc 100644 --- a/include/c.h +++ b/include/c.h @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include @@ -335,6 +337,28 @@ static inline size_t get_hostname_max(void) return 64; } + +static inline int drop_permissions(void) +{ + errno = 0; + + /* drop supplementary groups */ + if (setgroups(0, NULL) != 0) + goto fail; + + /* drop GID */ + if (setgid(getgid()) < 0) + goto fail; + + /* drop UID */ + if (setuid(getuid()) < 0) + goto fail; + + return 0; +fail: + return errno ? -errno : -1; +} + /* * The usleep function was marked obsolete in POSIX.1-2001 and was removed * in POSIX.1-2008. It was replaced with nanosleep() that provides more diff --git a/lib/canonicalize.c b/lib/canonicalize.c index e101c5b7a..6f85b47e5 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -170,8 +170,7 @@ char *canonicalize_path_restricted(const char *path) pipes[0] = -1; errno = 0; - /* drop permissions */ - if (setgid(getgid()) < 0 || setuid(getuid()) < 0) + if (drop_permissions() != 0) canonical = NULL; /* failed */ else { char *dmname = NULL; diff --git a/libblkid/src/topology/dm.c b/libblkid/src/topology/dm.c index 37fce6d62..b210a805b 100644 --- a/libblkid/src/topology/dm.c +++ b/libblkid/src/topology/dm.c @@ -73,10 +73,7 @@ static int probe_dm_tp(blkid_probe pr, if (dmpipe[1] != STDOUT_FILENO) dup2(dmpipe[1], STDOUT_FILENO); - /* The libblkid library could linked with setuid programs */ - if (setgid(getgid()) < 0) - exit(1); - if (setuid(getuid()) < 0) + if (drop_permissions() != 0) exit(1); snprintf(maj, sizeof(maj), "%d", major(devno)); diff --git a/libblkid/src/topology/lvm.c b/libblkid/src/topology/lvm.c index bd079d47b..8b0c0feea 100644 --- a/libblkid/src/topology/lvm.c +++ b/libblkid/src/topology/lvm.c @@ -82,10 +82,7 @@ static int probe_lvm_tp(blkid_probe pr, if (lvpipe[1] != STDOUT_FILENO) dup2(lvpipe[1], STDOUT_FILENO); - /* The libblkid library could linked with setuid programs */ - if (setgid(getgid()) < 0) - exit(1); - if (setuid(getuid()) < 0) + if (drop_permissions() != 0) exit(1); lvargv[0] = cmd; diff --git a/libmount/src/context_mount.c b/libmount/src/context_mount.c index 8c0a20e55..55ebf7945 100644 --- a/libmount/src/context_mount.c +++ b/libmount/src/context_mount.c @@ -645,10 +645,7 @@ static int exec_helper(struct libmnt_context *cxt) const char *args[14], *type; int i = 0; - if (setgid(getgid()) < 0) - _exit(EXIT_FAILURE); - - if (setuid(getuid()) < 0) + if (drop_permissions() != 0) _exit(EXIT_FAILURE); if (!mnt_context_switch_origin_ns(cxt)) diff --git a/libmount/src/context_umount.c b/libmount/src/context_umount.c index 57eda75be..173637a15 100644 --- a/libmount/src/context_umount.c +++ b/libmount/src/context_umount.c @@ -696,10 +696,7 @@ static int exec_helper(struct libmnt_context *cxt) const char *args[12], *type; int i = 0; - if (setgid(getgid()) < 0) - _exit(EXIT_FAILURE); - - if (setuid(getuid()) < 0) + if (drop_permissions() != 0) _exit(EXIT_FAILURE); if (!mnt_context_switch_origin_ns(cxt)) diff --git a/sys-utils/eject.c b/sys-utils/eject.c index ca5fbc380..457ce0d08 100644 --- a/sys-utils/eject.c +++ b/sys-utils/eject.c @@ -658,12 +658,8 @@ static void umount_one(const struct eject_control *ctl, const char *name) switch (fork()) { case 0: /* child */ - if (setgid(getgid()) < 0) - err(EXIT_FAILURE, _("cannot set group id")); - - if (setuid(getuid()) < 0) - err(EXIT_FAILURE, _("cannot set user id")); - + if (drop_permissions() != 0) + err(EXIT_FAILURE, _("drop permissions failed")); if (ctl->p_option) execl("/bin/umount", "/bin/umount", name, "-n", (char *)NULL); else diff --git a/sys-utils/mount.c b/sys-utils/mount.c index 0a85a2345..ce1de16dc 100644 --- a/sys-utils/mount.c +++ b/sys-utils/mount.c @@ -54,13 +54,8 @@ static void suid_drop(struct libmnt_context *cxt) const uid_t ruid = getuid(); const uid_t euid = geteuid(); - if (ruid != 0 && euid == 0) { - if (setgid(getgid()) < 0) - err(MNT_EX_FAIL, _("setgid() failed")); - - if (setuid(getuid()) < 0) - err(MNT_EX_FAIL, _("setuid() failed")); - } + if (ruid != 0 && euid == 0 && drop_permissions() != 0) + err(MNT_EX_FAIL, _("drop permissions failed")); /* be paranoid and check it, setuid(0) has to fail */ if (ruid != 0 && setuid(0) == 0) diff --git a/sys-utils/swapon.c b/sys-utils/swapon.c index 0f47d8516..da836c47e 100644 --- a/sys-utils/swapon.c +++ b/sys-utils/swapon.c @@ -333,13 +333,8 @@ static int swap_reinitialize(struct swap_device *dev) return -1; case 0: /* child */ - if (geteuid() != getuid()) { - /* in case someone uses swapon as setuid binary */ - if (setgid(getgid()) < 0) - exit(EXIT_FAILURE); - if (setuid(getuid()) < 0) - exit(EXIT_FAILURE); - } + if (geteuid() != getuid() && drop_permissions() != 0) + exit(EXIT_FAILURE); cmd[idx++] = "mkswap"; if (dev->label) { diff --git a/sys-utils/umount.c b/sys-utils/umount.c index ec357d0df..f5931767c 100644 --- a/sys-utils/umount.c +++ b/sys-utils/umount.c @@ -118,13 +118,8 @@ static void suid_drop(struct libmnt_context *cxt) const uid_t ruid = getuid(); const uid_t euid = geteuid(); - if (ruid != 0 && euid == 0) { - if (setgid(getgid()) < 0) - err(MNT_EX_FAIL, _("setgid() failed")); - - if (setuid(getuid()) < 0) - err(MNT_EX_FAIL, _("setuid() failed")); - } + if (ruid != 0 && euid == 0 && drop_permissions() != 0) + err(MNT_EX_FAIL, _("drop permissions failed")); /* be paranoid and check it, setuid(0) has to fail */ if (ruid != 0 && setuid(0) == 0) diff --git a/text-utils/more.c b/text-utils/more.c index fe5b1d245..3f45d1114 100644 --- a/text-utils/more.c +++ b/text-utils/more.c @@ -1250,12 +1250,9 @@ static void __attribute__((__format__ (__printf__, 3, 4))) } va_end(argp); - if (geteuid() != getuid() || getegid() != getgid()) { - if (setgid(getgid()) < 0) - err(EXIT_FAILURE, _("setgid failed")); - if (setuid(getuid()) < 0) - err(EXIT_FAILURE, _("setuid failed")); - } + if ((geteuid() != getuid() || getegid() != getgid()) + && drop_permissions() != 0) + err(EXIT_FAILURE, _("drop permissions failed")); execvp(cmd, args); errsv = errno;