summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSkyler Ferrante <sjf5462@rit.edu>2024-03-08 12:53:21 -0500
committerAlejandro Colomar <alx@kernel.org>2024-03-13 23:19:51 +0100
commitf4293f9fbc2b855878f549d9124bdd638fb08c60 (patch)
tree15a90a9f36561d1da91a4f762f69afcf764d8229
parent39192107a6afcd3fbe2f37eae3ab604a047f87ee (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.am1
-rw-r--r--lib/fd.c41
-rw-r--r--lib/prototypes.h3
-rw-r--r--src/chage.c7
-rw-r--r--src/chfn.c4
-rw-r--r--src/chsh.c1
-rw-r--r--src/expiry.c5
-rw-r--r--src/gpasswd.c2
-rw-r--r--src/newgrp.c3
-rw-r--r--src/passwd.c1
-rw-r--r--src/su.c2
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);
diff --git a/src/chfn.c b/src/chfn.c
index 2fd81d3d..213b0bb7 100644
--- a/src/chfn.c
+++ b/src/chfn.c
@@ -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);
diff --git a/src/chsh.c b/src/chsh.c
index e29ac580..c019e357 100644
--- a/src/chsh.c
+++ b/src/chsh.c
@@ -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);
diff --git a/src/su.c b/src/su.c
index 8c3be346..f9a18e25 100644
--- a/src/su.c
+++ b/src/su.c
@@ -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);