From 9df4801e0b65073cc8a9031b22a73532ef7fdc2c Mon Sep 17 00:00:00 2001 From: Vegard Nossum Date: Fri, 21 Jul 2023 14:55:19 +0200 Subject: [PATCH] newgrp: fix potential string injection Since newgrp is setuid-root, any write() system calls it does in order to print error messages will be done as the root user. Unprivileged users can get newgrp to print essentially arbitrary strings to any open file in this way by passing those strings as argv[0] when calling execve(). For example: $ setpid() { (exec -a $1$'\n:' newgrp '' 2>/proc/sys/kernel/ns_last_pid & wait) >/dev/null; } $ setpid 31000 $ readlink /proc/self 31001 This is not a vulnerability in newgrp; it is a bug in the Linux kernel. However, this type of bug is not new [1] and it makes sense to try to mitigate these types of bugs in userspace where possible. [1]: https://lwn.net/Articles/476947/ Signed-off-by: Vegard Nossum --- src/newgrp.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/newgrp.c b/src/newgrp.c index babb28e94..f786a96fa 100644 --- a/src/newgrp.c +++ b/src/newgrp.c @@ -417,11 +417,18 @@ int main (int argc, char **argv) * but we do not need to restore the previous process persona and we * don't need to re-exec anything. -- JWP */ - Prog = Basename (argv[0]); + + /* + * Ensure that "Prog" is always either "newgrp" or "sg" to avoid + * injecting arbitrary strings into our stderr/stdout, as this can + * be an exploit vector. + */ + is_newgrp = (strcmp (Basename (argv[0]), "newgrp") == 0); + Prog = is_newgrp ? "newgrp" : "sg"; + log_set_progname(Prog); log_set_logfd(stderr); - is_newgrp = (strcmp (Prog, "newgrp") == 0); - OPENLOG (is_newgrp ? "newgrp" : "sg"); + OPENLOG (Prog); argc--; argv++; -- 2.47.2