diff options
author | Skyler Ferrante <sjf5462@rit.edu> | 2024-03-08 12:53:21 -0500 |
---|---|---|
committer | Alejandro Colomar <alx@kernel.org> | 2024-03-13 23:19:51 +0100 |
commit | f4293f9fbc2b855878f549d9124bdd638fb08c60 (patch) | |
tree | 15a90a9f36561d1da91a4f762f69afcf764d8229 | |
parent | 39192107a6afcd3fbe2f37eae3ab604a047f87ee (diff) |
lib/, src/: Add checks for fd omission
Adding function check_fds to new file fd.c. The function check_fds
should be called in every setuid/setgid program.
Co-developed-by: Alejandro Colomar <alx@kernel.org>
Cherry-picked-from: d2f2c1877a30 ("Adding checks for fd omission")
Link: <https://github.com/shadow-maint/shadow/pull/964>
Link: <https://inbox.sourceware.org/libc-alpha/ZeyujhVRsDTUNUtw@debian/T/>
[alx: It seems we shouldn't need this, as libc does it for us. But it ]
[ shouldn't hurt either. Let's be paranoic. ]
Cc: <Guillem Jover <guillem@hadrons.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: "Skyler Ferrante (RIT Student)" <sjf5462@rit.edu>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: Rich Felker <dalias@libc.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Thorsten Glaser <tg@mirbsd.de>
Cc: NRK <nrk@disroot.org>
Cc: Florian Weimer <fweimer@redhat.com>
Cc: enh <enh@google.com>
Cc: Laurent Bercot <ska-dietlibc@skarnet.org>
Cc: Gabriel Ravier <gabravier@gmail.com>
Cc: Zack Weinberg <zack@owlfolio.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
-rw-r--r-- | lib/Makefile.am | 1 | ||||
-rw-r--r-- | lib/fd.c | 41 | ||||
-rw-r--r-- | lib/prototypes.h | 3 | ||||
-rw-r--r-- | src/chage.c | 7 | ||||
-rw-r--r-- | src/chfn.c | 4 | ||||
-rw-r--r-- | src/chsh.c | 1 | ||||
-rw-r--r-- | src/expiry.c | 5 | ||||
-rw-r--r-- | src/gpasswd.c | 2 | ||||
-rw-r--r-- | src/newgrp.c | 3 | ||||
-rw-r--r-- | src/passwd.c | 1 | ||||
-rw-r--r-- | src/su.c | 2 |
11 files changed, 63 insertions, 7 deletions
diff --git a/lib/Makefile.am b/lib/Makefile.am index 7db960c8..1146e39b 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -53,6 +53,7 @@ libshadow_la_SOURCES = \ faillog.h \ failure.c \ failure.h \ + fd.c \ fields.c \ find_new_gid.c \ find_new_uid.c \ diff --git a/lib/fd.c b/lib/fd.c new file mode 100644 index 00000000..bcfa374a --- /dev/null +++ b/lib/fd.c @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: 2024, Skyler Ferrante <sjf5462@rit.edu> +// SPDX-License-Identifier: BSD-3-Clause + +/** + * To protect against file descriptor omission attacks, we open the std file + * descriptors with /dev/null if they are not already open. Code is based on + * fix_fds from sudo.c. + */ + +#include <fcntl.h> +#include <stdlib.h> +#include <unistd.h> + +#include "prototypes.h" + +static void check_fd(int fd); + +void +check_fds(void) +{ + /** + * Make sure stdin, stdout, stderr are open + * If they are closed, set them to /dev/null + */ + check_fd(STDIN_FILENO); + check_fd(STDOUT_FILENO); + check_fd(STDERR_FILENO); +} + +static void +check_fd(int fd) +{ + int devnull; + + if (fcntl(fd, F_GETFL, 0) != -1) + return; + + devnull = open("/dev/null", O_RDWR); + if (devnull != fd) + abort(); +} diff --git a/lib/prototypes.h b/lib/prototypes.h index cacf3d21..b9d4fbdf 100644 --- a/lib/prototypes.h +++ b/lib/prototypes.h @@ -127,6 +127,9 @@ extern void initenv (void); extern void set_env (int, char *const *); extern void sanitize_env (void); +/* fd.c */ +extern void check_fds (void); + /* fields.c */ extern void change_field (char *, size_t, const char *); extern int valid_field (const char *, const char *); diff --git a/src/chage.c b/src/chage.c index b64961f6..18ad80eb 100644 --- a/src/chage.c +++ b/src/chage.c @@ -762,13 +762,12 @@ int main (int argc, char **argv) gid_t rgid; const struct passwd *pw; - /* - * Get the program name so that error messages can use it. - */ + sanitize_env (); + check_fds (); + log_set_progname(Prog); log_set_logfd(stderr); - sanitize_env (); (void) setlocale (LC_ALL, ""); (void) bindtextdomain (PACKAGE, LOCALEDIR); (void) textdomain (PACKAGE); @@ -616,10 +616,12 @@ int main (int argc, char **argv) char new_gecos[BUFSIZ]; /* buffer for new GECOS fields */ char *user; + sanitize_env (); + check_fds (); + log_set_progname(Prog); log_set_logfd(stderr); - sanitize_env (); (void) setlocale (LC_ALL, ""); (void) bindtextdomain (PACKAGE, LOCALEDIR); (void) textdomain (PACKAGE); @@ -473,6 +473,7 @@ int main (int argc, char **argv) const struct passwd *pw; /* Password entry from /etc/passwd */ sanitize_env (); + check_fds (); log_set_progname(Prog); log_set_logfd(stderr); diff --git a/src/expiry.c b/src/expiry.c index 673cbc3c..1900335e 100644 --- a/src/expiry.c +++ b/src/expiry.c @@ -123,11 +123,12 @@ int main (int argc, char **argv) struct passwd *pwd; struct spwd *spwd; + sanitize_env (); + check_fds (); + log_set_progname(Prog); log_set_logfd(stderr); - sanitize_env (); - /* * Start by disabling all of the keyboard signals. */ diff --git a/src/gpasswd.c b/src/gpasswd.c index 34205cc7..a3fa80d0 100644 --- a/src/gpasswd.c +++ b/src/gpasswd.c @@ -956,6 +956,8 @@ int main (int argc, char **argv) #endif sanitize_env (); + check_fds (); + (void) setlocale (LC_ALL, ""); (void) bindtextdomain (PACKAGE, LOCALEDIR); (void) textdomain (PACKAGE); diff --git a/src/newgrp.c b/src/newgrp.c index 68e80fe9..051b5675 100644 --- a/src/newgrp.c +++ b/src/newgrp.c @@ -390,6 +390,9 @@ int main (int argc, char **argv) #ifdef WITH_AUDIT audit_help_open (); #endif + + check_fds (); + (void) setlocale (LC_ALL, ""); (void) bindtextdomain (PACKAGE, LOCALEDIR); (void) textdomain (PACKAGE); diff --git a/src/passwd.c b/src/passwd.c index 3ef0cf3f..b9873b06 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -730,6 +730,7 @@ int main (int argc, char **argv) const struct spwd *sp; /* Shadow file entry for user */ sanitize_env (); + check_fds (); log_set_progname(Prog); log_set_logfd(stderr); @@ -999,6 +999,8 @@ int main (int argc, char **argv) int ret; #endif /* USE_PAM */ + check_fds (); + (void) setlocale (LC_ALL, ""); (void) bindtextdomain (PACKAGE, LOCALEDIR); (void) textdomain (PACKAGE); |