From: Christian Brauner <brauner@kernel.org>
To: Rich Felker <dalias@libc.org>
Cc: Ariadne Conill <ariadne@dereferenced.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Eric Biederman <ebiederm@xmission.com>,
Kees Cook <keescook@chromium.org>,
Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH] fs/exec: require argv[0] presence in do_execveat_common()
Date: Wed, 26 Jan 2022 15:46:03 +0100 [thread overview]
Message-ID: <20220126144603.vlxmlngqlpsjtmzw@wittgenstein> (raw)
In-Reply-To: <20220126132729.GA7942@brightrain.aerifal.cx>
On Wed, Jan 26, 2022 at 08:27:30AM -0500, Rich Felker wrote:
> On Wed, Jan 26, 2022 at 04:39:47AM +0000, Ariadne Conill wrote:
> > The first argument to argv when used with execv family of calls is
> > required to be the name of the program being executed, per POSIX.
>
> That's not quite the story. The relevant text is a "should", meaning
> that to be "strictly conforming" an application has to follow the
> convention, but still can't assume its invoker did. (Note that most
> programs do not aim to be "strictly conforming"; it's not just the
> word strictly applied as an adjective to conforming, but a definition
> of its own imposing very stringent portability conditions beyond what
> the standard already imposes.) Moreover, POSIX (following ISO C, after
> this was changed from early C drafts) rejected making it a
> requirement. This is documented in the RATIONALE for execve:
>
> Early proposals required that the value of argc passed to main()
> be "one or greater". This was driven by the same requirement in
> drafts of the ISO C standard. In fact, historical implementations
> have passed a value of zero when no arguments are supplied to the
> caller of the exec functions. This requirement was removed from
> the ISO C standard and subsequently removed from this volume of
> POSIX.1-2017 as well. The wording, in particular the use of the
> word should, requires a Strictly Conforming POSIX Application to
> pass at least one argument to the exec function, thus guaranteeing
> that argc be one or greater when invoked by such an application.
> In fact, this is good practice, since many existing applications
> reference argv[0] without first checking the value of argc.
>
> Source: https://pubs.opengroup.org/onlinepubs/9699919799/functions/execve.html
>
> Note that despite citing itself as POSIX.1-2017 above, this is not a
> change in the 2017 edition; it's just the way they self-cite. As far
> as I can tell, the change goes back to prior to the first publication
> of the standard.
>
> > By validating this in do_execveat_common(), we can prevent execution
> > of shellcode which invokes execv(2) family syscalls with argc < 1,
> > a scenario which is disallowed by POSIX, thus providing a mitigation
> > against CVE-2021-4034 and similar bugs in the future.
> >
> > The use of -EFAULT for this case is similar to other systems, such
> > as FreeBSD and OpenBSD.
>
> I don't like this choice of error, since in principle EFAULT should
> never happen when you haven't invoked memory-safety-violating UB.
> Something like EINVAL would be more appropriate. But if the existing
> practice for systems that do this is to use EFAULT, it's probably best
> to do the same thing.
>
> > Interestingly, Michael Kerrisk opened an issue about this in 2008,
> > but there was no consensus to support fixing this issue then.
> > Hopefully now that CVE-2021-4034 shows practical exploitative use
> > of this bug in a shellcode, we can reconsider.
>
> I'm not really opposed to attempting to change this with consensus
> (like, actually proposing it on the Austin Group tracker), but a less
> invasive change would be just enforcing it for the case where exec is
> a privilege boundary (suid/sgid/caps). There's really no motivation
> for changing longstanding standard behavior in a
> non-privilege-boundary case.
Agreed. If we do this at all then this has way less regression potential.
next prev parent reply other threads:[~2022-01-26 14:46 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-26 4:39 [PATCH] fs/exec: require argv[0] presence in do_execveat_common() Ariadne Conill
2022-01-26 6:42 ` Kees Cook
2022-01-26 7:28 ` Kees Cook
2022-01-26 11:18 ` Ariadne Conill
2022-01-26 12:33 ` Heikki Kallasjoki
2022-01-26 23:57 ` Kees Cook
2022-01-27 0:20 ` Eric W. Biederman
2022-01-26 16:59 ` David Laight
2022-01-26 13:27 ` Rich Felker
2022-01-26 14:46 ` Christian Brauner [this message]
2022-01-26 17:37 ` Ariadne Conill
2022-02-01 20:54 ` hypervis0r
2022-01-26 15:02 Alexey Dobriyan
2022-01-27 0:00 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220126144603.vlxmlngqlpsjtmzw@wittgenstein \
--to=brauner@kernel.org \
--cc=ariadne@dereferenced.org \
--cc=dalias@libc.org \
--cc=ebiederm@xmission.com \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.