Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: Improved the Windows example and added more wording about why it's an improvement.

...

Code Block
bgColor#ffcccc
langc
#include <stdio.h>
 
void func(const char *file_name) {
  FILE *file;

  if ((file = fopen(file_name, "wb")) == NULL) {
    /* Handle error */
  }

  /* Operate on the file. */

  fclose(file);
}

Compliant Solution (POSIX)

...

Code Block
bgColor#ccccff
langc
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#ifdef O_NOFOLLOW
  #define OPEN_FLAGS O_NOFOLLOW | O_NONBLOCK
#else
  #define OPEN_FLAGS O_NONBLOCK
#endif

void func(const char *file_name) {
  struct stat orig_st;
  struct stat open_st;
  int fd;
  int flags;

  if ((lstat(file_name, &orig_st) != 0) ||
      (!S_ISREG(orig_st.st_mode))) {
    /* Handle error */
  }

  /* A TOCTOU race condition exists here, see below. */

  fd = open(file_name, OPEN_FLAGS | O_WRONLY);
  if (fd == -1) {
    /* Handle error */
  }

  if (fstat(fd, &open_st) != 0) {
    /* Handle error */
  }

  if ((orig_st.st_mode != open_st.st_mode) ||
      (orig_st.st_ino  != open_st.st_ino) ||
      (orig_st.st_dev  != open_st.st_dev)) {
    /* The Filefile was tampered with. */
  }

  /* Optional: drop the O_NONBLOCK now that we are sure
     this is a good file. */
  if ((flags = fcntl(fd, F_GETFL)) == -1) {
    /* Handle error */
  }

  if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) != 0) {
    /* Handle error */
  }

  /* Operate on the file. */

  close(fd);
}

This code contains an intractable TOCTOU (time-of-check, time-of-use) race condition under which an attacker can alter the file referenced by file_name following the call to lstat() but before the call to open(). The switch will be discovered after the file is opened, but opening the file cannot be prevented in the case where this action itself causes undesired behavior.

...

Compliant Solution (Windows)

The GetFileType() function can be used to determine if the file is a disk fileMicrosoft documents a list of reserved identifiers that represent devices, as well as have a device namespace to be used specifically by devices [MSDN].  This compliant solution tests the given file name against these constructs:

Code Block
bgColor#ccccff
langc
#include <Windows#include <ctype.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <stdio.h>
 
voidstatic bool funcisReservedName(const TCHARchar *file_namepath) {
  /* This list of reserved names comes from  HANDLE hFile = CreateFile(file_name,
                            GENERIC_READ | GENERIC_WRITE, 0, 
                            NULL, OPEN_EXISTING,
                            FILE_ATTRIBUTE_NORMAL, NULL);
  if (hFile == INVALID_HANDLE_VALUE) {
    /* Handle error */
  } else if (GetFileType(hFile) != FILE_TYPE_DISK) {
    /* Handle error */
    CloseHandle(hFile);
  } else {
    /* Operate on file */
    CloseHandle(hFile);
  }
}MSDN. */
  static const char *reserved[] = { "nul", "con", "prn", "aux",
                                    "com1", "com2", "com3",
                                    "com4", "com5", "com6",
                                    "com7", "com8", "com9",
                                    "lpt1", "lpt2", "lpt3",
                                    "lpt4", "lpt5", "lpt6",
                                    "lpt7", "lpt8", "lpt9" };
  char *lower;
  char *temp;
  bool ret = false;
  
  /* First, check to see if this is a device namespace, which
     always starts with \\.\, since device namespaces are not
     legal file paths. */
  temp = strstr(path, "\\\\.\\");
  if (temp == path) {
    return true;
  }
 
  /* Since Windows uses a case insensitive file system, operate
     on a lowercase version of the given filename. Note: this
     ignores globalization issues and assumes ASCII
     characters. */
  lower = (char *)malloc(strlen(path) + 1);
  if (!lower) {
    return false;
  }
  
  temp = lower;
  while (*path) {
    *lower++ = tolower(*path++);
  }
  lower = temp;
 
  /* Compare against the list of ancient reserved names. */
  for (size_t i = 0; !ret &&
       i < sizeof(reserved) / sizeof(*reserved); ++i) {    
    if (0 == strcmp(lower, reserved[i])) {
      ret = true;
    }
  }
 
  free(lower);
  return ret;
} 

While it may be tempting to use the Win32 GetFileType() function, it is a dangerous API to use in this case. If the file name given identifies a named pipe, and that pipe is currently blocking on a read request, the call to GetFileType() will block until the read request completes. That allows an attacker to effectively launch a denial-of-service attack on your application. Furthermore, the act of opening a file handle may cause further action to be taken, such as line states being set to their default voltage when opening a serial device.

Risk Assessment

Allowing operations that are appropriate only for files to be performed on devices can result in denial-of-service attacks or more serious exploits depending on the platform.

...

...

[Garfinkel 1996]Section 5.6, "Device Files"
[Howard 2002]Chapter 11, "Canonical Representation Issues"
[Open Group 2004]open()

 

...