mkdir()/chmod() TOCTOU race condition under gcc linux

109 views Asked by At

I have gcc 'C' code running in RHEL 8.x / 9.x that has been flagged as having a 'TOCTOU race condition'.

It does a mkdir() followed by a chmod() and SonarQube points at the chmod().

I assume it is complaining that someone could do bad things between the mkdir() and the chmod() resulting in a substituted file or symlink getting 'chmod()'ed instead of the new directory I created.

The reason I do the subsequent chmod() is because the umask value will alter the permissions I specified on the mkdir(), so the chmod() is there to 'fix up' the file permissions to what I originally asked for.

Since my code is part of a larger system and is called by various programs as an entry in a shared object, I don't have the ability to modify umask as it would alter the umask for the whole process and any other thread that might be running at the same time could have its file creation behavior altered. (I don't have control over any other threads that may be running from the same process, or the umask that was used to start a given process).

I am under the impression that I can't create a directory with open() (which would have allowed me to use fchmod() on the opened handle and presumably avoid the race condition).

Is there some 'standard' way to avoid this TOCTOU race?

The 'create sub-directory' code is really only called if an attempt to create a file in said sub-directory fails with an 'ENOENT' (assuming that the failure reason is that the sub-directory doesn't exist).

(I'm trying to create this sub-directory in a '/tmp' type directory that gets erased on every system re-boot. The files I want to create will go in my new sub-directory, and the sub-directory is expected to need creation the first time my code is called after a reboot).

Something like the simplified:

#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>

int create_subdir(char *temp_dir_path);

   int create_subdir(char *temp_dir_path)

   {  /* create_subdir */

   int rc;                         /* Return code                                                          */

      rc = 0;

      /***************************************************************/
      /* Does the temp directory already exist within the parent     */
      /* directory? Try to create it to find out                     */
      /***************************************************************/

      rc = mkdir(temp_dir_path, S_IRWXU | S_IRWXG | S_IRWXO );

      if (rc == 0)
      {  /***************************************************************/
         /* We created it.                                              */
         /* Try and fix up any missing mode bits the umask may have     */
         /* prevented from getting set.                                 */
         /***************************************************************/

         rc = chmod(temp_dir_path, S_IRWXU | S_IRWXG | S_IRWXO);

      }

      return(rc);

   } /* create_subdir */


int main (int argc, char *argv[])
/**************************************************************/
/*                                                            */
/*    test mainline                                           */
/*                                                            */
/**************************************************************/

{ /* Main */

char temp_dir_path[128];
char f_name[128];

int fd, rc;

   strcpy(temp_dir_path, "/tmp/new-dir");
   strcpy(f_name, "/tmp/new-dir/xyzzy");

   fd = open( f_name,
              O_CREAT | O_RDWR | O_TRUNC,
              S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);

   if (fd == -1)
   {
      if (errno == ENOENT)
      {  /***************************************************************/
         /* Presume this is because the sub-directory under /tmp        */
         /* doesn't yet exist. Try to create it.                        */
         /***************************************************************/

         rc = create_subdir(temp_dir_path);
      }
   }

} /* main */

Edit: Added a 'fuller' simplified 'C' version.

Based on the comments so far, I'll give doing mkdir() with '0' permissions while ensuring the 'parent' directory has the sticky bit enabled a try. (The parent directory already had the sticky bit set in my case).

2

There are 2 answers

3
zwol On

The reason I do the subsequent chmod() is because the umask value will alter the permissions I specified on the mkdir(), so the chmod() is there to 'fix up' the file permissions to what I originally asked for.

If you use 0000 as the permission argument to mkdir, then the new directory will be completely inaccessible to everyone (except possibly root, but root is trusted anyway) in between its creation and when the chmod happens, so you don't need to worry about anyone taking advantage of incorrect permissions on the newly created directory.

As far as I know, here is no system call (not even in the *at family) that creates a directory and returns you an open O_PATH/O_SEARCH fd for that directory. Therefore, there is still a race if you use 0000: someone could swoop in and modify the parent directory in between the mkdir and the chmod, causing you to chmod the wrong thing. I don't think there is any way to avoid this race. There are, however, two workarounds that should be airtight:

  • If it's possible to make the parent directory be owned and writable only by root, then have root create the directory you need and chown it to the appropriate user.
  • If the parent directory must be world-writable, then make sure it is also "sticky" (mode 1777); then a process running as some other user cannot rename or delete your directory. However, a process running as some other user could create your directory before you get around to doing it; you can mitigate this by checking the ownership on "your" directory using stat or fstat.
0
Jonathan Leffler On

Are you sure you need to override the umask value? You could check whether the value will interfere with your required mode and not set the mode again if the umask value won't interfere.

Assuming that you must set the mode, you can use the open() function to open the directory that will hold the new directory, either with O_SEARCH or O_RDONLY. You can then use mkdirat() with that file descriptor as the “at” argument and a simple name (no slashes) for the directory name.

Consider whether it is safe to set mode_t old = umask(); …create directory…; umask(old); to set the permissions accurately without needing to use chmod(). In a multithreaded application, that may not be a good choice; the umask value is global state, like the current directory is global state.

Otherwise, create the directory with 0 for the mode and then chmod() it. You might even need to use openat() to open the new directory and fchmod() to set the mode.

Remember to close all the opened file descriptors.