Austin Group Defect Tracker

Aardvark Mark IV


Viewing Issue Simple Details Jump to Notes ] Issue History ] Print ]
ID Category Severity Type Date Submitted Last Update
0000555 [1003.1(2008)/Issue 7] System Interfaces Editorial Clarification Requested 2012-04-15 15:56 2019-06-10 08:55
Reporter nenolod View Status public  
Assigned To ajosey
Priority normal Resolution Accepted As Marked  
Status Closed  
Name William Pitcock
Organization
User Reference
Section fclose
Page Number 805
Line Number 26808
Interp Status ---
Final Accepted Text See Note: 0001240
Summary 0000555: the incestuous relationship of fclose(stderr) and STDERR_FILENO
Description hi,

on some systems such as freebsd, and anything glibc-based (linux, hurd etc.), if:

1. you fclose(stderr) closing both stderr FILE stream and underlying STDERR_FILENO descriptor;

2. accept() a new client of SOCK_STREAM type later which is assigned by the kernel the descriptor STDERR_FILENO;

3. have a memory-related error later on.

then the socket with fd=STDERR_FILENO may get information from the system libc about the memory error, resulting in private information leaks.

this happens because the libc calls something like:

char errbuf[] = "*** memory error here ***";
write(STDERR_FILENO, errbuf, strlen(errbuf));

the usual way around this is to use a pattern like the following:

{
    int fd;

    fclose(stdin);
    fclose(stdout);
    fclose(stderr);

    fd = open("/dev/null", O_RDWR);
    dup2(fd, STDIN_FILENO);
    dup2(fd, STDOUT_FILENO);
    dup2(fd, STDERR_FILENO);
    close(fd);
}

which remaps the descriptors used for stdio to /dev/null.
Desired Action if stdio FILE streams are closed, then should it not be considered invalid behaviour to write to their associated STDIN_FILENO/STDOUT_FILENO/STDERR_FILENO once the respective FILE streams are closed?

if not, then it may be a good idea to explicitly document this pattern as an example in the specification of dup2() so that daemon authors are encouraged to secure stdio descriptors appropriately when daemonizing.

an alternative approach without probably any specification stability impact: change fclose() specification so it should automatically dup2 the fd to /dev/null instead of calling close() on it.
Tags c99, tc2-2008
Attached Files

- Relationships
related to 0000173Closedajosey Executing programs with "bad" file descriptors 0, 1 or 2 
related to 0000662Under Reviewajosey Clarify or add file descriptor preservation and atomicity requirements for freopen 

-  Notes
(0001201)
Don Cragun (manager)
2012-04-15 16:43

I assume that the first call to fclose() in the example in this bug's
Description field was intended to be "fclose(stdin);" instead of
"fclose(stdio);".

Note that the last paragraph of the description of fclose() (P805,
L26808) already says: "After the call to fclose(), any use of stream
results in undefined behavior." and the details in XSH section 2.5.1
Interaction of File Descriptors and Standard IO Streams (P491-493,
L16827-16897 indirectly says the behavior is unspecified if you
fclose(stderr) and then write to STDERR_FILENO. So writing to
STDERR_FILENO after calling close(stderr) is indeed invalid behavior.
(0001202)
nenolod (reporter)
2012-04-15 16:52

Hi,

Yes, indeed that was a typo.

I believe the specification should be updated then to either explicitly define fclose(stdin/stdout/stderr) without a subsequent dup2() and fdopen() as undefined behaviour (which would make glibc and freebsd memory debugging output on direct FDs still invalid) or perhaps mandate that fclose() dup2 file descriptors to /dev/null when the underlying FD number is STDIO_FILENO, STDOUT_FILENO or STDERR_FILENO.

The behaviour of the latter would ensure that there is no information leak.

The behaviour of the former would ensure that there is no information leak and also allow daemons the ability to use 3 more file descriptors. In some cases (ircd, jabberd, etc.) where there are many connections, these additional descriptors may be useful.
(0001203)
mitr (reporter)
2012-04-15 23:17

"So writing to STDERR_FILENO after calling close(stderr) is indeed invalid behavior."

The issue here is writing to STDERR_FILENO by the implementation, without any way for the application to control it.

close(STDERR_FILENO) and subsequent file descriptor allocation have a well-defined behavior that portable programs rely on, so mandating anything to the effect that STDERR_FILENO should immediately be reopened with something like /dev/null is probably not possible. The case for fclose(stderr) is perhaps a little less clear-cut, but I still think that making fclose(stderr) so different from close(STDERR_FILENO) is not desirable.


So, it seems to me that adding a paragraph to "APPLICATION USAGE" warning about this is the best thing to do here.
(0001204)
Don Cragun (manager)
2012-04-15 23:49

If an implementation has some standard function in libc that writes to stderr (using an STDIO stream interface) or to STDERR_FILENO (using a file descriptor interface) when the standard doesn't specify that output will be sent to that file stream or file descriptor as stated in Note: 0001203, the implementation does not conform to the standard (unless the user gave an option at build time or at run time that enabled this extra debugging feature). If the user specified such an option at build time, then the application writer is responsible to be sure that stderr (and the underlying STDERR_FILENO) are open whenever an interface producing that debugging information is called. If a user specified such an open at run time, they have created a non-conforming environment for the execution of conforming applications (unless the application's documentation explicitly states that it is OK to use that non-standard debugging option).

Adding an APPLICATION USAGE note specifying that allowing an application to be run in a non-conforming environment may produce undefined behavior could be added to fclose(). But if we go down that road, the same note could appear in the APPLICATION USAGE section of every function in the standard.
(0001205)
nenolod (reporter)
2012-04-16 01:00

Hi,

I think the most realistic approach is to explicitly document the open() + dup2() + fdopen() combination most daemons use to secure stdio descriptors.

However, I think that this should actually be unnecessary, as a conforming implementation should just fwrite(stderr, ...) instead of write(STDERR_FILENO, ...).

Regardless, glibc's __libc_message() implementation is opt-out, so it's certainly not conformant -- it will try /dev/tty, and then STDERR_FILENO respectively, without checking if STDERR_FILENO is actually stderr anymore.

Regarding that, I am not sure what a good approach for checking if STDERR_FILENO is still stderr would be, but that is offtopic here.
(0001207)
ldwyer (reporter)
2012-04-16 18:32

If the implementation does the following ...

    #include <stdio.h>
    ...
    int err_fd;
    ...
    /* enable MUTEX here */

    err_fd = fileno(stderr);
    if ( ! (err_fd < 0) ){
    write(err_fd, message, message_len);
    }

    /* disable MUTEX here */
    ...

... it will at least be correct for the case where the application
does fclose(stderr) followed by an open().

To make this "case" thread safe, it should be protected by a MUTEX that
prevents another thread from executing an open/dup/dup2/fcntl (directly
or via a stdio function).

In the case of a system daemon, this discussion is moot anyway. The
developer of the daemon is in control and should not be executing
fclose(stderr). It should instead do freopen("/dev/null",mode,stderr).
(0001239)
eblake (manager)
2012-05-10 15:55
edited on: 2012-05-10 15:58

The issue is broader than just the glibc message printed on a double free (which only happens after undefined behavior in the program). There are well-defined interfaces which are impacted: From the C standard, at least assert() and perror() are required to write to the standard error stream, and would be impacted after either an fclose(stderr) (per C standard) or close(STDERR_FILENO) (per POSIX). There are also POSIX interfaces such as getopt() that are required to write to stderr.

(0001240)
nick (manager)
2012-05-10 16:31
edited on: 2012-05-24 15:48

Change Application Usage for fclose (page 805 line 26839) from "None." to:

Since after the call to fclose() any use of stream results in undefined behavior, fclose() should not be used on stdin, stdout, or stderr except immediately before process termination (see XBD 3.297 on page 81), so as to avoid triggering undefined behavior in other standard interfaces that rely on these streams. If there are any atexit() handlers registered by the application, such a call to fclose() should not occur until the last handler is finishing. Once fclose() has been used to close stdin, stdout, or stderr, there is no standard way to reopen any of these streams.

Use of freopen() to change stdin, stdout, or stderr instead of closing them avoids the danger of a file unexpectedly being opened as one of the special file descriptors STDIN_FILENO, STDOUT_FILENO, or STDERR_FILENO at a later time in the application.

===

Add to Application Usage for close (page 678 line 22962):

Usage of close() on file descriptors STDIN_FILENO, STDOUT_FILENO or STDERR_FILENO should immediately be followed by an operation to reopen these file descriptors. Unexpected behavior will result if any of these file descriptors is left in a closed state (for example, an EBADF error from perror()) or if an unrelated open() or similar call later in the application accidentally allocates a file to one of these well-known file descriptors. Furthermore, a close() followed by a reopen operation (e.g. open(), dup() etc) is not atomic; dup2() should be used to change standard file descriptors.


- Issue History
Date Modified Username Field Change
2012-04-15 15:56 nenolod New Issue
2012-04-15 15:56 nenolod Status New => Under Review
2012-04-15 15:56 nenolod Assigned To => ajosey
2012-04-15 15:56 nenolod Name => William Pitcock
2012-04-15 15:56 nenolod Section => fclose
2012-04-15 15:56 nenolod Page Number => using HTML version which lacks page numbers
2012-04-15 15:56 nenolod Line Number => using HTML version which lacks line numbers
2012-04-15 16:40 nenolod Summary fclose(stderr) and STDERR_FILENO => the incestuous relationship of fclose(stderr) and STDERR_FILENO
2012-04-15 16:43 Don Cragun Page Number using HTML version which lacks page numbers => 805
2012-04-15 16:43 Don Cragun Line Number using HTML version which lacks line numbers => 26808
2012-04-15 16:43 Don Cragun Interp Status => ---
2012-04-15 16:43 Don Cragun Note Added: 0001201
2012-04-15 16:43 Don Cragun Summary the incestuous relationship of fclose(stderr) and STDERR_FILENO => fclose(stderr) and STDERR_FILENO
2012-04-15 16:47 nenolod Description Updated
2012-04-15 16:47 nenolod Desired Action Updated
2012-04-15 16:51 Don Cragun Summary fclose(stderr) and STDERR_FILENO => the incestuous relationship of fclose(stderr) and STDERR_FILENO
2012-04-15 16:52 nenolod Note Added: 0001202
2012-04-15 23:17 mitr Note Added: 0001203
2012-04-15 23:49 Don Cragun Note Added: 0001204
2012-04-16 01:00 nenolod Note Added: 0001205
2012-04-16 18:32 ldwyer Note Added: 0001207
2012-05-10 15:41 eblake Relationship added related to 0000173
2012-05-10 15:54 eblake Tag Attached: c99
2012-05-10 15:55 eblake Note Added: 0001239
2012-05-10 15:58 eblake Note Edited: 0001239
2012-05-10 16:31 nick Note Added: 0001240
2012-05-10 16:32 nick Final Accepted Text => See Note: 0001240
2012-05-10 16:32 nick Status Under Review => Resolved
2012-05-10 16:32 nick Resolution Open => Accepted As Marked
2012-05-10 16:32 nick Tag Attached: tc2-2008
2012-05-24 15:23 nick Note Edited: 0001240
2012-05-24 15:25 nick Note Edited: 0001240
2012-05-24 15:48 nick Note Edited: 0001240
2013-02-21 18:09 eblake Relationship added related to 0000662
2019-06-10 08:55 agadmin Status Resolved => Closed


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker