Fix: sessiond: TOCTOU error on save of session configuration

The session_save() function checks for the existance and access rights
on the target session configuration filename before opening it. This
results in a TOCTOU (Time of check, time of use) problem.

Defer the check and error reporting to the run_as_open() call.

1191754 Time of check time of use
An attacker could change the filename's file association or other
attributes between the check and use.  In save_session: A check occurs
on a file's attributes before the file is used in a privileged
operation, but things may have changed (CWE-367)

Reported-by: Coverity Scan
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-sessiond/save.c b/src/bin/lttng-sessiond/save.c
index 3c4c3b2bce527045f7c1c9d3c09f00773fd68a07..cdeb4004d6b4f99e8ef5c6b5a99f9d9f55dd2c7a 100644 (file)
--- a/src/bin/lttng-sessiond/save.c
+++ b/src/bin/lttng-sessiond/save.c
@@ -2290,6 +2290,7 @@ int save_session(struct ltt_session *session,
        struct config_writer *writer = NULL;
        size_t session_name_len;
        const char *provided_path;
        struct config_writer *writer = NULL;
        size_t session_name_len;
        const char *provided_path;

+       int file_open_flags = O_CREAT | O_WRONLY | O_TRUNC;

 
        assert(session);
        assert(attr);
 
        assert(session);
        assert(attr);


@@ -2363,18 +2364,26 @@ int save_session(struct ltt_session *session,
        len += sizeof(DEFAULT_SESSION_CONFIG_FILE_EXTENSION);
        config_file_path[len] = '\0';
 
        len += sizeof(DEFAULT_SESSION_CONFIG_FILE_EXTENSION);
        config_file_path[len] = '\0';
 

-       if (!access(config_file_path, F_OK) && !attr->overwrite) {
-               /* File exists, notify the user since the overwrite flag is off. */
-               ret = LTTNG_ERR_SAVE_FILE_EXIST;
-               goto end;
+       if (!attr->overwrite) {
+               file_open_flags |= O_EXCL;

        }
 
        }
 

-       fd = run_as_open(config_file_path, O_CREAT | O_WRONLY | O_TRUNC,
+       fd = run_as_open(config_file_path, file_open_flags,

                S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP,
                LTTNG_SOCK_GET_UID_CRED(creds), LTTNG_SOCK_GET_GID_CRED(creds));
        if (fd < 0) {
                PERROR("Could not create configuration file");
                S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP,
                LTTNG_SOCK_GET_UID_CRED(creds), LTTNG_SOCK_GET_GID_CRED(creds));
        if (fd < 0) {
                PERROR("Could not create configuration file");

-               ret = LTTNG_ERR_SAVE_IO_FAIL;
+               switch (errno) {
+               case EEXIST:
+                       ret = LTTNG_ERR_SAVE_FILE_EXIST;
+                       break;
+               case EACCES:
+                       ret = LTTNG_ERR_EPERM;
+                       break;
+               default:
+                       ret = LTTNG_ERR_SAVE_IO_FAIL;
+                       break;
+               }

                goto end;
        }
 
                goto end;
        }