Page History

...

Noncompliant Code Example

Wiki MarkupIn this noncompliant code example derived from a [vulnerability|BB. Definitions#vulnerability] in OpenBSD's {{chpass}} program \[ [NAI 98|AA. Bibliography#NAI 98]\], a file containing sensitive data is opened for reading. The program then retrieves the registered editor from the {{EDITOR}} environment variable and executes it using the {{system()}} command. If, the {{system()}} command is implemented in a way that spawns a child process, then the child process inherits the file descriptors opened by its parent. As a result, the child process, which in this example is the program specified by the {{EDITOR}} environment variable, will be able to access the contents of the potentially sensitive file called {{file_name}}.

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

/* Initialize file_name */

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 */
}

...

int flags;
char *editor;
char *file_name;

/* Initialize file_name */

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 greater than or equal to 2.6.23) 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 08|AA. Bibliography#Austin Group 08]\]. 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.

char *editor;
char *file_name;

/* Initialize file_name */

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 */
}

...

This rule appears in the Java Secure Coding Standard as FIO04-J. Release resources when they are no longer needed.

Bibliography

...

\[[Austin Group 08|AA. Bibliography#Austin Group 08]\] \[[Dowd 06|AA. Bibliography#Dowd 06]\] Chapter 10, "UNIX Processes" (File Descriptor Leaks 582-587) \[[MITRE 07|AA. Bibliography#MITRE 07]\] [CWE-404|http://cwe.mitre.org/data/definitions/404.html], "Improper Resource Shutdown or Release," and [CWE-403|http://cwe.mitre.org/data/definitions/403.html], "UNIX File Descriptor Leak," [CWE-770|http://cwe.mitre.org/data/definitions/770.html], "Allocation of Resources Without Limits or Throttling" \[[MSDN|AA. Bibliography#MSDN]\] [Inheritance|http://msdn.microsoft.com/en-us/library/ms683463.aspx] (Windows) \[[NAI 98|AA. Bibliography#NAI 98]\]
[Dowd 06] Chapter 10, "UNIX Processes" (File Descriptor Leaks 582-587)
[MITRE 07] CWE-404, "Improper Resource Shutdown or Release," and CWE-403, "UNIX File Descriptor Leak," CWE-770, "Allocation of Resources Without Limits or Throttling"
[MSDN] Inheritance (Windows)
[NAI 98]

...

FIO41-CPP. Do not call getc() or putc() with stream arguments that have side effects      09. Input Output (FIO)      FIO43-CPP. Do not create temporary files in shared directories