From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id ED76A298C0 for ; Tue, 12 Mar 2024 04:23:47 +0100 (CET) Received: (qmail 9638 invoked by uid 550); 12 Mar 2024 03:19:35 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 9618 invoked from network); 12 Mar 2024 03:19:35 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710213814; x=1710818614; darn=lists.openwall.com; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=tMCtnw+BNi55tyBLK9KyjR7gIYbz1NxaDGVxDwgc1TE=; b=SqSkPLIPGyiGRJKC44oII9dFXuX/j9ysZpbh+yJJYQPZonsx7aGDivTZv9RhHSeB4T xJtbznGskMJzTZT/+PUkgC6fQdn/n53ElZlna6e7lJmCsB03M0hZQgmSHg/5RAEV4zQC G+ngXMIlUxvcQOsHLQeCVPcDpkIgR6t9rkNbSBUcHafkXD1KYw+ndJJOdb6rlnX/L0iH k7mRa0FhUoWLcX4wRbpe9Dsazlaacv5duTZZTsidJlo6gfwL97PF0xiLesMMOfAQtrSD ji60dGqnLVoq5SdxqOmlxX0fiMP7wFVhWDc6CuTNOY77Bunykg47hTgPPktbrpkRMTaa tu1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710213814; x=1710818614; h=in-reply-to:from:content-language:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=tMCtnw+BNi55tyBLK9KyjR7gIYbz1NxaDGVxDwgc1TE=; b=NGR53HT2Elgpeem4SHFp/9jnSqrPRov3CJBHn9Odlc5wmS/dbD1wKrRdmeUeH34llm dhc3wlJyhT48+DUdxyQFm+o2hgx3Avk9/usFMFeNxiBFpOxXcMTkmTC18lNaw86dZi3L rJ1L+IfaMmJXDddnNHYBn7TE4lIONO2VR+J8X4ejP414xOd2yJXYZRHJwRjErkZbwq87 QwquRxeNFEUeh8OpDANiZF/LC+7wEsPnwusdKrjPIJ8z/Dnv73+JDa41srkkMoCttyHP iO5PRbFHmFsJrO0kVM2qkAOGC82M/OBcQo5PzW9mCrOSxZnPHGJhZ+SgRnAnEGoC3bu4 jOCQ== X-Forwarded-Encrypted: i=1; AJvYcCXbPtwZ66v8k0uSXI7/m7ZhU3fBuaNoAJQDRJpl5NhkeOaoLRzQpFx5vqOPeLdq4dKCJqj275zxxNO9QbOtablX/C0PBsfpQA== X-Gm-Message-State: AOJu0YwPVnc83P70dkZJeGs7aCxj+rjFLLrZ7K9VYb/Yw07Inbe8F/fK yWYVmPusN/Y5kvnOvdRAJfW7zH/cmWDGzo3d3rZ9zSOpT/acB+GD X-Google-Smtp-Source: AGHT+IFt6C7vDLVvPtRXNEexYzfSVXBN7FNmWGcxdhr6MGyxplqG+scaA83UlRA/Juad/H5X/RlKcg== X-Received: by 2002:a05:600c:b8c:b0:413:3391:2f23 with SMTP id fl12-20020a05600c0b8c00b0041333912f23mr461423wmb.35.1710213814000; Mon, 11 Mar 2024 20:23:34 -0700 (PDT) Content-Type: multipart/alternative; boundary="------------l7DgbejBlotUyGvbDpeFTNKP" Message-ID: <80ae776d-8a62-4fa0-99b1-9524ad90ad6c@gmail.com> Date: Tue, 12 Mar 2024 03:23:32 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Rich Felker Cc: "Skyler Ferrante (RIT Student)" , Andreas Schwab , Alejandro Colomar , Thorsten Glaser , musl@lists.openwall.com, NRK , Guillem Jover , libc-alpha@sourceware.org, libbsd@lists.freedesktop.org, "Serge E. Hallyn" , Iker Pedrosa , Christian Brauner References: <20240310193956.GU4163@brightrain.aerifal.cx> <20240310234410.GW4163@brightrain.aerifal.cx> <20240311194756.GY4163@brightrain.aerifal.cx> <40962405-c5b4-4925-9ca5-7a1c723ebbfd@gmail.com> <20240312004309.GZ4163@brightrain.aerifal.cx> Content-Language: en-US From: Gabriel Ravier In-Reply-To: <20240312004309.GZ4163@brightrain.aerifal.cx> Subject: Re: [musl] Re: Tweaking the program name for functions This is a multi-part message in MIME format. --------------l7DgbejBlotUyGvbDpeFTNKP Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 --------------l7DgbejBlotUyGvbDpeFTNKP Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
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

--------------l7DgbejBlotUyGvbDpeFTNKP--