From d2f2c1877a30849912e1f3490a21808fa87fcd4c Mon Sep 17 00:00:00 2001 From: Skyler Ferrante Date: Fri, 8 Mar 2024 12:53:21 -0500 Subject: [PATCH] Adding 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 --- lib/Makefile.am | 1 + lib/fd.c | 41 +++++++++++++++++++++++++++++++++++++++++ lib/prototypes.h | 3 +++ src/chage.c | 7 +++---- src/chfn.c | 4 +++- src/chsh.c | 1 + src/expiry.c | 5 +++-- src/gpasswd.c | 2 ++ src/newgrp.c | 3 +++ src/passwd.c | 1 + src/su.c | 2 ++ 11 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 lib/fd.c diff --git a/lib/Makefile.am b/lib/Makefile.am index 538385726..86c6be73d 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -61,6 +61,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 000000000..bcfa374a2 --- /dev/null +++ b/lib/fd.c @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: 2024, Skyler Ferrante +// 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 +#include +#include + +#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 0fac291c6..ff4f7f996 100644 --- a/lib/prototypes.h +++ b/lib/prototypes.h @@ -120,6 +120,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 522681064..c29ef99e5 100644 --- a/src/chage.c +++ b/src/chage.c @@ -768,13 +768,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); diff --git a/src/chfn.c b/src/chfn.c index 6dc982901..0877ab7e8 100644 --- a/src/chfn.c +++ b/src/chfn.c @@ -620,10 +620,12 @@ int main (int argc, char **argv) char *user; const struct passwd *pw; + 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); diff --git a/src/chsh.c b/src/chsh.c index ce9b16299..9be52e3db 100644 --- a/src/chsh.c +++ b/src/chsh.c @@ -472,6 +472,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 792f33f20..12647a23a 100644 --- a/src/expiry.c +++ b/src/expiry.c @@ -125,11 +125,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 9692e2511..de6b1c4cc 100644 --- a/src/gpasswd.c +++ b/src/gpasswd.c @@ -930,6 +930,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 c536e084b..1b3d76b83 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 2b77c2761..63729f82d 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -728,6 +728,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); diff --git a/src/su.c b/src/su.c index ea0e7223c..80c085971 100644 --- a/src/su.c +++ b/src/su.c @@ -1007,6 +1007,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);