Make correct changes to deal with toctou issues (CIDs below) (#4793)

1271307, 1445217: use *at() calls to work around toctou

diff --git a/src/lib/server/cf_file.c b/src/lib/server/cf_file.c
index 6e02ffbd08c80e230802d4b3d6fb794c82ba17c7..909e0542a36089c5d28caacf69cb75dcea1524ac 100644 (file)
--- a/src/lib/server/cf_file.c
+++ b/src/lib/server/cf_file.c
@@ -40,6 +40,7 @@ RCSID("$Id$")
 #include <freeradius-devel/util/misc.h>
 #include <freeradius-devel/util/perm.h>
 #include <freeradius-devel/util/syserror.h>
+#include <freeradius-devel/util/file.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -529,12 +530,22 @@ static int cf_file_open(CONF_SECTION *cs, char const *filename, bool from_dir, F
         */
        if (from_dir) {
                cf_file_t my_file;
+               char const *r;
+               int dirfd;
 
                my_file.cs = cs;
                my_file.filename = filename;
 
-               /* coverity[fs_check_call] */
-               if (stat(filename, &my_file.buf) < 0) goto error;
+               /*
+                *      Find and open the directory containing filename so we can use
+                *       the "at"functions to avoid time of check/time of use insecurities.
+                */
+               if (fr_dirfd(&dirfd, &r, filename) < 0) {
+                       ERROR("Failed to open directory containing %s", filename);
+                       return -1;
+               }
+
+               if (fstatat(dirfd, r, &my_file.buf, 0) < 0) goto error;
 
                file = fr_rb_find(tree, &my_file);
 
@@ -547,20 +558,26 @@ static int cf_file_open(CONF_SECTION *cs, char const *filename, bool from_dir, F
                 *      However, if the file WAS read from a wildcard
                 *      $INCLUDE directory, then we read it again.
                 */
-               if (file && !file->from_dir) return 1;
+               if (file && !file->from_dir) {
+                       if (dirfd != AT_FDCWD) close(dirfd);
+                       return 1;
+               }
+               fd = openat(dirfd, r, O_RDONLY, 0);
+               fp = (fd < 0) ? NULL : fdopen(fd, "r");
+               if (dirfd != AT_FDCWD) close(dirfd);
+       } else {
+               fp = fopen(filename, "r");
+               if (fp) fd = fileno(fp);
        }
 
        DEBUG2("including configuration file %s", filename);
 
-       fp = fopen(filename, "r");
        if (!fp) {
        error:
                ERROR("Unable to open file \"%s\": %s", filename, fr_syserror(errno));
                return -1;
        }
 
-       fd = fileno(fp);
-
        MEM(file = talloc(tree, cf_file_t));
 
        file->filename = talloc_strdup(file, filename); /* The rest of the code expects this to be a talloced buffer */

diff --git a/src/lib/util/file.c b/src/lib/util/file.c
index 0cd84a787c04458d87b6733db290bf986f4a97c5..4272102331d0b93f51043b1dc640dd2511023f4b 100644 (file)
--- a/src/lib/util/file.c
+++ b/src/lib/util/file.c
@@ -378,3 +378,34 @@ char const *fr_cwd_strip(char const *filename)
 
        return filename;
 }
+
+/** From a pathname, return fd and filename needed for *at() functions
+ *
+ * @param[in] dirfd    points to place to store the dirfd
+ * @param[in] filename points to placd to store a pointer into pathname
+ *                     that points to the filename
+ * @param[in] pathname the full pathname of the file
+ *
+ * @return
+ *     - -1 on error
+ *     -  0 on success
+ */
+int fr_dirfd(int *dirfd, char const **filename, char const *pathname)
+{
+       char const *last_slash = strrchr(pathname, '/');
+
+       if (last_slash == NULL) {
+               *filename = pathname;
+               *dirfd = AT_FDCWD;
+               return 0;
+       }
+       {
+               char dirpath[(last_slash - pathname) + 1];
+
+               memcpy(dirpath, pathname, last_slash - pathname);
+               dirpath[last_slash - pathname] = '\0';
+               *filename = last_slash + 1;
+               *dirfd = open(dirpath, O_DIRECTORY);
+               return (*dirfd < 0) ? -1 : 0;
+       }
+}

diff --git a/src/lib/util/file.h b/src/lib/util/file.h
index 357e756d336c7c8ddda855d30c253ef9dd008e55..29d3b16c0693a372be2a70eed20a5bbf4be4910e 100644 (file)
--- a/src/lib/util/file.h
+++ b/src/lib/util/file.h
@@ -55,6 +55,8 @@ int           fr_unlink(char const *filename);
 
 char const     *fr_cwd_strip(char const *filename);
 
+int            fr_dirfd(int *dirfd, char const **filename, char const *pathname) CC_HINT(nonnull);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/src/listen/control/proto_control_unix.c b/src/listen/control/proto_control_unix.c
index 97417e01c89fcf12a6f5ea85398905217029bd4f..6f520fbd6d41b238967b480afa41c46e2f47a3a5 100644 (file)
--- a/src/listen/control/proto_control_unix.c
+++ b/src/listen/control/proto_control_unix.c
@@ -28,6 +28,7 @@
 #include <freeradius-devel/server/protocol.h>
 #include <freeradius-devel/util/perm.h>
 #include <freeradius-devel/util/trie.h>
+#include <freeradius-devel/util/file.h>
 #include <netdb.h>
 
 #include "proto_control.h"
@@ -384,6 +385,8 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
 #endif
 {
        int sockfd;
+       int dirfd;
+       char const *r;
        size_t len;
        socklen_t socklen;
        struct sockaddr_un salocal;
@@ -411,14 +414,23 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
 
        socklen = SUN_LEN(&salocal);
 
+       /*
+        *      Find and open the directory containing path so we can use the "at"
+        *      functions to avoid time of check/time of use insecurities.
+        */
+       if (fr_dirfd(&dirfd, &r, path) < 0) {
+               fr_strerror_printf("Failed to open directory containing %s", path);
+               return -1;
+       }
+
        /*
         *      Check the path.
         */
-       /* coverity[fs_check_call] */
-       if (stat(path, &buf) < 0) {
+       if (fstatat(dirfd, r, &buf, 0) < 0) {
                if (errno != ENOENT) {
                        fr_strerror_printf("Failed to stat %s: %s", path, fr_syserror(errno));
                        close(sockfd);
+                       if (dirfd != AT_FDCWD) close(dirfd);
                        return -1;
                }
 
@@ -435,6 +447,7 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
                        ) {
                        fr_strerror_printf("Cannot turn %s into socket", path);
                        close(sockfd);
+                       if (dirfd != AT_FDCWD) close(dirfd);
                        return -1;
                }
 
@@ -444,6 +457,7 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
                if (buf.st_uid != geteuid()) {
                        fr_strerror_printf("We do not own %s", path);
                        close(sockfd);
+                       if (dirfd != AT_FDCWD) close(dirfd);
                        return -1;
                }
 
@@ -456,12 +470,14 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
                        fr_strerror_printf("Control socket '%s' is already in use", path);
                        close(client_fd);
                        close(sockfd);
+                       if (dirfd != AT_FDCWD) close(dirfd);
                        return -1;
                }
 
-               if (unlink(path) < 0) {
+               if (unlinkat(dirfd, r, 0) < 0) {
                       fr_strerror_printf("Failed to delete %s: %s", path, fr_syserror(errno));
                       close(sockfd);
+                      if (dirfd != AT_FDCWD) close(dirfd);
                       return -1;
                }
        }
@@ -469,6 +485,7 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
        if (bind(sockfd, (struct sockaddr *)&salocal, socklen) < 0) {
                fr_strerror_printf("Failed binding to %s: %s", path, fr_syserror(errno));
                close(sockfd);
+               if (dirfd != AT_FDCWD) close(dirfd);
                return -1;
        }
 
@@ -476,12 +493,15 @@ static int fr_server_domain_socket_peercred(char const *path, uid_t UNUSED uid,
         *      FIXME: There's a race condition here.  But Linux
         *      doesn't seem to permit fchmod on domain sockets.
         */
-       if (chmod(path, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) < 0) {
+       if (fchmodat(dirfd, r, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP, 0) < 0) {
                fr_strerror_printf("Failed setting permissions on %s: %s", path, fr_syserror(errno));
                close(sockfd);
+               if (dirfd != AT_FDCWD) close(dirfd);
                return -1;
        }
 
+       if (dirfd != AT_FDCWD) close(dirfd);
+
        if (listen(sockfd, 8) < 0) {
                fr_strerror_printf("Failed listening to %s: %s", path, fr_syserror(errno));
                close(sockfd);