]> git.ipfire.org Git - thirdparty/shadow.git/commitdiff
newgrp: fix potential string injection
authorVegard Nossum <vegard.nossum@oracle.com>
Fri, 21 Jul 2023 12:55:19 +0000 (14:55 +0200)
committerSerge Hallyn <serge@hallyn.com>
Sat, 22 Jul 2023 04:32:19 +0000 (23:32 -0500)
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 <vegard.nossum@oracle.com>
src/newgrp.c

index babb28e94bad5c65581272be38678be5e8c1be2b..f786a96fad703bd8debca1ddfd8d78d3ddbad6a6 100644 (file)
@@ -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++;