[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <80ae776d-8a62-4fa0-99b1-9524ad90ad6c@gmail.com>
Date: Tue, 12 Mar 2024 03:23:32 +0000
From: Gabriel Ravier <gabravier@...il.com>
To: Rich Felker <dalias@...c.org>
Cc: "Skyler Ferrante (RIT Student)" <sjf5462@....edu>,
Andreas Schwab <schwab@...e.de>, Alejandro Colomar <alx@...nel.org>,
Thorsten Glaser <tg@...bsd.de>, musl@...ts.openwall.com,
NRK <nrk@...root.org>, Guillem Jover <guillem@...rons.org>,
libc-alpha@...rceware.org, libbsd@...ts.freedesktop.org,
"Serge E. Hallyn" <serge@...lyn.com>, Iker Pedrosa <ipedrosa@...hat.com>,
Christian Brauner <christian@...uner.io>
Subject: Re: Re: Tweaking the program name for <err.h> functions
On 3/12/24 00:43, Rich Felker wrote:
> On Tue, Mar 12, 2024 at 12:18:24AM +0000, Gabriel Ravier wrote:
>> On 3/11/24 19:47, Rich Felker wrote:
>>> On Mon, Mar 11, 2024 at 11:30:04AM -0400, Skyler Ferrante (RIT Student) wrote:
>>>> Hmm, maybe I'm missing something, but it seems you can close(fd) for
>>>> the standard fds and then call execve, and the new process image will
>>>> have no fd 0,1,2. I've tried this on a default Ubuntu 22.04 system.
>>>> This seems to affect shadow-utils and other setuid/setgid binaries.
>>>>
>>>> Here is a repo I built for testing,
>>>> https://github.com/skyler-ferrante/fd_omission/. What is the correct
>>>> glibc behavior? Am I misunderstanding something?
>>> As Florian noted, you're missing that strace cannot invoke it suid.
>>> POSIX explicitly permits the implementation to open these fds if they
>>> started closed in suid execs, and IIRC indicates as a future direction
>>> that it might be permitted for all execs. We do the same in musl in
>>> the suid case. So really the only way that "writing attacker
>>> controlled prefix strings to fd 2" becomes an issue is if the
>>> application erroneously closes fd 2 and lets something else get opened
>>> on it.
>>>
>>> (Aside: making _FORTIFY_SOURCE>1 trap close(n) with n<3 would be an
>>> interesting idea... :)
>> Doing this would break many programs, such as:
>> - most of coreutils, e.g. programs like ls, cat or head, since they
>> always `close` their input and output descriptors (when they've
>> written or read something) to make sure to diagnose all errors
>> - grep
>> - xargs
>> - find
> This makes it so they can malfunction during exit when it
> flushes/closes the corresponding stdio FILEs. If nothing else has been
> opened in the mean time, under typical implementations it should be
> safe, but I think per 2.5.1 Interaction of File Descriptors and
> Standard I/O Streams:
>
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_05_01
>
> it's undefined.
>
> The safe way to do what they want is to dup the fd they want to
> close-and-check-for-errors, open /dev/null, dup2 that over the
> original fd, then close the first dup.
>
> Or, don't exit()/return-from-main, but instead _exit, so there's no
> subsequent access to the FILE.
Those applications above (though some of those below appear to do raw
/close/ calls) all circumvent your objection by calling /fclose/ on the
standard streams rather than /close/-ing the file descriptors directly,
which seems legal according to POSIX given otherwise the following quote
would make no sense:
> 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
Section 3.287, on page 73), 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.
- POSIX, /System Interfaces/, *fclose( )*, *APPLICATION USAGE*
and all of those applications listed above do the /fclose/ at the end of
their /atexit/ handlers - the wording implies the fact they return from
that /atexit/ handler when the /fclose/ succeeds is fine too since it's
done when "the last handler is finishing" (i.e. after all other usage of
standard interfaces which may use the standard streams)), which seems to
imply calling /exit/ after /fclose/-ing one of the standard streams
should be legal.
Furthermore, the description of /close/ also states:
> 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.
- POSIX, /System Interfaces/, *fclose( )*, *APPLICATION USAGE*
which simply points out "unexpected" (which does not appear to mean
"undefined") behavior along with examples of some of the potential
consequences and points out replacing those descriptors after the
/close/ is not atomic and that /dup2/ should be used instead - but
"should" is not the same word as "must", so this seems to implicitly
allow not using /dup2/ or even not reopening the file descriptors at all
- which seems accurate to me, I can't see anything in the standard,
including in that *Interaction of File Descriptors and Standard I/O
Streams* section you mentioned earlier, that would result in undefined
behavior upon /close/-ing the file descriptors before /fclose/, except
that that section seems to indicate the application must not leave data
buffered in the stream (which does not seem to be something any of the
applications I've mentioned before or after this do - though it's not
like I've engaged in exhaustive program analysis to ensure this is the
case either, so that's more of an assumption than anything else, but the
scenarios in which they close these streams (near the beginning or
termination of the application) seem like they should make it quite
likely this is not the case).
It seems to me as though further use of a /FILE/ after its file
descriptor was /close/-ed would simply result in [EBADF] errors
(...which could obviously also easily lead to issues involving having
multiple active handles on the same file if someone else was to call
/open/ afterwards, but that's a separate issue).
>
>> - strace, which (using the half-closed self-pipe trick mentioned
>> earlier in this thread to avoid reusing them later btw) closes the
>> standard descriptors, to avoid changing the behavior of programs
>> calling it if e.g. its input is a pipe (where if it left the fds
>> open that'd mean the writer would get SIGPIPE later than if the
>> program was ran without strace)
>> - tcsh, which deliberately does `close(n)` with `n < 3` to make it
>> so all the standard FDs point to `/dev/null`
>> - troff and groff (and thus man)
>> - git
>> - many more... I have found these by simply stracing random programs
>> as found on my system with `ls /bin/ | shuf -n1`
> Yes, I'm quite aware it's commonplace, but it would be something nice
> to get cleaned up...
>
> Rich
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
