Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

Code Block
bgColor#FFcccc
langc
FILE *f;
#include <stdio.h>
#include <stdlib.h>
 
void func(const char *editor;
char *file_name;

/* Initialize file_name */

file_name) {
  FILE *f;
  const char *editor;

  f = fopen(file_name, "r");
  if (f == NULL) {
    /* Handle fopen() error */
  }
 
/* ... */
editor = getenv("EDITOR");
  if (editor == NULL) {
    /* Handle getenv() error */
  }
 
  if (system(editor) == -1) {
    /* Handle error */
  }
}

On UNIX-based systems, child processes are typically spawned using a form of fork() and exec(), and the child process always inherits from its parent any file descriptors that do not have the close-on-exec flag set. Under Microsoft Windows, the CreateProcess() function is typically used to start a child process. In Windows, file-handle inheritance is determined on a per-file basis. Additionally, the CreateProcess() function itself provides a mechanism to limit file-handle inheritance. As a result, the child process spawned by CreateProcess() may not receive copies of the parent process's open file handles.

...

Code Block
bgColor#ccccff
langc
FILE* f;
#include <stdio.h>
#include <stdlib.h>
 
void func(const char *editor;
char *file_name;

/* Initialize file_name */

file_name) {
  FILE *f;
  const char *editor;

  f = fopen(file_name, "r");
  if (f == NULL) {
    /* Handle fopen() error */
  }
/* ... */
  fclose(f);
  f = NULL;
  
  editor = getenv("EDITOR");
  if (editor == NULL) {
    /* Handle getenv() error */
  }
 
  /* Sanitize environment before calling system()!. */
  if (system(editor) == -1) {
    /* Handle Errorerror */
  }
}

Several security issues remain in this example. Compliance with recommendations such as STR02-C. Sanitize data passed to complex subsystems and FIO02-C. Canonicalize path names originating from untrusted sources is necessary to prevent exploitation. However, these recommendations do not address the specific issue of file descriptor leakage discussed here.

...

Code Block
bgColor#ccccff
langc
int flags;
char *editor;
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
 
void func(const char *file_name;

/* Initialize file_name */

) {
  int flags;
  char *editor;

  int fd = open(file_name, O_RDONLY);
  if (fd == -1) {
    /* Handle error */
  }

  flags = fcntl(fd, F_GETFD);
  if (flags == -1) {
    /* Handle error */
  }

  if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) {
    /* Handle error */
  }

/* ... */

editor = getenv("EDITOR");
  if (editor == NULL) {
    /* Handle getenv() error */
  }
 
  if (system(editor) == -1) {
    /* Handle error */
  }
}

Some systems (such as those with Linux kernel versions 2.6.23 and greater) have an O_CLOEXEC flag that provides the close-on-exec function directly in open(). This flag is required by POSIX.1-2008 [Austin Group 2008]. In multithreaded programs, this flag should be used, if possible, because it prevents a timing hole between open() and fcntl() when using FD_CLOEXEC, during which another thread can create a child process while the file descriptor does not have close-on-exec set.

Code Block
bgColor#ccccff
langc
char *editor;
#include <stdio.h>
#include <stdlib.h>
 
void func(const char *file_name;

/* Initialize file_name */

) {
  char *editor;
  int fd = open(file_name, O_RDONLY | O_CLOEXEC);
  if (fd == -1) {
    /* Handle error */
  }
 
/* ... */

editor = getenv("EDITOR");
  if (editor == NULL) {
    /* Handle getenv() error */
  }
 
  if (system(editor) == -1) {
    /* Handle error */
  }
}

Risk Assessment

Failing to properly close files may allow unintended access to, or exhaustion of, system resources.

...