From: Max Kellermann Date: Sat, 10 May 2025 20:26:50 +0000 (+0200) Subject: lib/env, ...: use getauxval(AT_SECURE) for SUID check X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b36add06585acf77e1a50fc0d2c901a0129582a4;p=thirdparty%2Futil-linux.git lib/env, ...: use getauxval(AT_SECURE) for SUID check Comparing effective and real uid/gid is not a proper way to check for SUID execution: 1. this does not consider file capabilities 2. this check breaks when NO_NEW_PRIVS is used as the Linux kernel resets effective ids during execve(); this means the check is false, but the process still has raised capabilities For more details about the NO_NEW_PRIVS problem, check this post and the surrounding thread: https://lore.kernel.org/lkml/20250509184105.840928-1-max.kellermann@ionos.com/ Signed-off-by: Max Kellermann --- diff --git a/include/debug.h b/include/debug.h index 15b09d07a..a59de3442 100644 --- a/include/debug.h +++ b/include/debug.h @@ -36,6 +36,7 @@ #include #include +#include // for getauxval() struct ul_debug_maskname { const char *name; @@ -89,7 +90,7 @@ struct ul_debug_maskname { } else \ lib ## _debug_mask = mask; \ if (lib ## _debug_mask) { \ - if (getuid() != geteuid() || getgid() != getegid()) { \ + if (getauxval(AT_SECURE)) { \ lib ## _debug_mask |= __UL_DEBUG_FL_NOADDR; \ fprintf(stderr, "%d: %s: don't print memory addresses (SUID executable).\n", getpid(), # lib); \ } \ diff --git a/lib/env.c b/lib/env.c index 0874fe482..3fc4f2e21 100644 --- a/lib/env.c +++ b/lib/env.c @@ -16,6 +16,7 @@ #include #endif #include +#include // for getauxval() #include #include "env.h" @@ -260,7 +261,7 @@ void sanitize_env(void) char *safe_getenv(const char *arg) { - if ((getuid() != geteuid()) || (getgid() != getegid())) + if (getauxval(AT_SECURE)) return NULL; #ifdef HAVE_PRCTL if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) == 0) diff --git a/libmount/src/context.c b/libmount/src/context.c index 15a8ad3bb..84e98aa3f 100644 --- a/libmount/src/context.c +++ b/libmount/src/context.c @@ -42,6 +42,7 @@ #include "match.h" #include +#include // for getauxval() #include #include "mount-api-utils.h" @@ -55,14 +56,13 @@ struct libmnt_context *mnt_new_context(void) { struct libmnt_context *cxt; - uid_t ruid, euid; + uid_t ruid; cxt = calloc(1, sizeof(*cxt)); if (!cxt) return NULL; ruid = getuid(); - euid = geteuid(); mnt_context_reset_status(cxt); @@ -77,7 +77,7 @@ struct libmnt_context *mnt_new_context(void) INIT_LIST_HEAD(&cxt->hooksets_datas); /* if we're really root and aren't running setuid */ - cxt->restricted = (uid_t) 0 == ruid && ruid == euid ? 0 : 1; + cxt->restricted = (uid_t) 0 == ruid && !getauxval(AT_SECURE) ? 0 : 1; cxt->noautofs = 0; diff --git a/login-utils/chfn.c b/login-utils/chfn.c index 7067ffaf2..adfa3d63a 100644 --- a/login-utils/chfn.c +++ b/login-utils/chfn.c @@ -29,6 +29,7 @@ #include #include #include +#include // for getauxval() #include #include @@ -269,7 +270,7 @@ static void get_login_defs(struct chfn_control *ctl) int broken = 0; /* real root does not have restrictions */ - if (geteuid() == getuid() && getuid() == 0) { + if (!getauxval(AT_SECURE) && getuid() == 0) { ctl->allow_fullname = ctl->allow_room = ctl->allow_work = ctl->allow_home = 1; return; } @@ -449,7 +450,7 @@ int main(int argc, char **argv) #ifdef HAVE_LIBUSER /* If we're setuid and not really root, disallow the password change. */ - if (geteuid() != getuid() && uid != ctl.pw->pw_uid) { + if (getauxval(AT_SECURE) && uid != ctl.pw->pw_uid) { #else if (uid != 0 && uid != ctl.pw->pw_uid) { #endif diff --git a/login-utils/chsh.c b/login-utils/chsh.c index 19f091534..490d51864 100644 --- a/login-utils/chsh.c +++ b/login-utils/chsh.c @@ -29,6 +29,7 @@ #include #include #include +#include // for getauxval() #include #include @@ -243,7 +244,7 @@ int main(int argc, char **argv) /* reality check */ #ifdef HAVE_LIBUSER /* If we're setuid and not really root, disallow the password change. */ - if (geteuid() != getuid() && uid != pw->pw_uid) { + if (getauxval(AT_SECURE) && uid != pw->pw_uid) { #else if (uid != 0 && uid != pw->pw_uid) { #endif diff --git a/login-utils/su-common.c b/login-utils/su-common.c index cf10caa6f..2df10ee31 100644 --- a/login-utils/su-common.c +++ b/login-utils/su-common.c @@ -23,6 +23,7 @@ */ #include #include +#include // for getauxval() #include #include #include @@ -939,10 +940,9 @@ static void load_config(void *data) static int is_not_root(void) { const uid_t ruid = getuid(); - const uid_t euid = geteuid(); /* if we're really root and aren't running setuid */ - return (uid_t) 0 == ruid && ruid == euid ? 0 : 1; + return (uid_t) 0 == ruid && !getauxval(AT_SECURE) ? 0 : 1; } /* Don't rely on PAM and reset the most important limits. */ diff --git a/sys-utils/swapon.c b/sys-utils/swapon.c index c761b85d0..0a8c40407 100644 --- a/sys-utils/swapon.c +++ b/sys-utils/swapon.c @@ -20,6 +20,7 @@ #include #include #include +#include // for getauxval() #include #include #include @@ -348,7 +349,7 @@ static int swap_reinitialize(struct swap_device *dev) return -1; case 0: /* child */ - if (geteuid() != getuid() && drop_permissions() != 0) + if (getauxval(AT_SECURE) && drop_permissions() != 0) exit(EXIT_FAILURE); cmd[idx++] = "mkswap"; diff --git a/term-utils/wall.c b/term-utils/wall.c index 22c3918bb..fbd8e54fb 100644 --- a/term-utils/wall.c +++ b/term-utils/wall.c @@ -42,6 +42,7 @@ * */ +#include // for getauxval() #include #include #include @@ -414,7 +415,7 @@ static char *makemsg(char *fname, char **mvec, int mvecsz, * instead of "wall file". */ uid_t uid = getuid(); - if (uid && (uid != geteuid() || getgid() != getegid())) + if (uid && getauxval(AT_SECURE)) errx(EXIT_FAILURE, _("will not read %s - use stdin."), fname); diff --git a/text-utils/more.c b/text-utils/more.c index 1ddfcaa4c..22f01ba24 100644 --- a/text-utils/more.c +++ b/text-utils/more.c @@ -50,6 +50,7 @@ #include #include #include +#include // for getauxval() #include #include #include @@ -1273,7 +1274,7 @@ static void __attribute__((__format__ (__printf__, 3, 4))) } va_end(argp); - if ((geteuid() != getuid() || getegid() != getgid()) + if (getauxval(AT_SECURE) && drop_permissions() != 0) err(EXIT_FAILURE, _("drop permissions failed"));